From b70e0b06dfdf221031022e9595bc401b1a24f90f Mon Sep 17 00:00:00 2001 From: Denis Kasak Date: Thu, 29 Jul 2021 12:15:38 +0200 Subject: [PATCH] Differentiate between malformed pickle objects and trailing junk data. Adds the OLM_PICKLE_EXTRA_DATA error code. We fail with this code when the pickle object looks right except for some unexpected trailing bytes which we didn't process. --- include/olm/error.h | 6 ++++++ src/error.c | 3 ++- src/inbound_group_session.c | 2 +- src/olm.cpp | 4 ++-- src/outbound_group_session.c | 2 +- src/pk.cpp | 2 +- tests/test_group_session.cpp | 4 ++-- tests/test_olm.cpp | 4 ++-- tests/test_pk.cpp | 2 +- 9 files changed, 18 insertions(+), 11 deletions(-) diff --git a/include/olm/error.h b/include/olm/error.h index 17e819d..37c46d6 100644 --- a/include/olm/error.h +++ b/include/olm/error.h @@ -58,6 +58,12 @@ enum OlmErrorCode { */ OLM_SAS_THEIR_KEY_NOT_SET = 16, + /** + * The pickled object was successfully decoded, but the unpickling still failed + * because it had some extraneous junk data at the end. + */ + OLM_PICKLE_EXTRA_DATA = 17, + /* remember to update the list of string constants in error.c when updating * this list. */ }; diff --git a/src/error.c b/src/error.c index 7d39637..6775eee 100644 --- a/src/error.c +++ b/src/error.c @@ -32,7 +32,8 @@ static const char * ERRORS[] = { "BAD_LEGACY_ACCOUNT_PICKLE", "BAD_SIGNATURE", "OLM_INPUT_BUFFER_TOO_SMALL", - "OLM_SAS_THEIR_KEY_NOT_SET" + "OLM_SAS_THEIR_KEY_NOT_SET", + "OLM_PICKLE_EXTRA_DATA" }; const char * _olm_error_to_string(enum OlmErrorCode error) diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c index 421c17b..d6f73b7 100644 --- a/src/inbound_group_session.c +++ b/src/inbound_group_session.c @@ -275,7 +275,7 @@ size_t olm_unpickle_inbound_group_session( if (pos != end) { /* Input was longer than expected. */ - session->last_error = OLM_CORRUPTED_PICKLE; + session->last_error = OLM_PICKLE_EXTRA_DATA; return (size_t)-1; } diff --git a/src/olm.cpp b/src/olm.cpp index d0cf653..660b21b 100644 --- a/src/olm.cpp +++ b/src/olm.cpp @@ -303,7 +303,7 @@ size_t olm_unpickle_account( return std::size_t(-1); } else if (pos != end) { /* Input was longer than expected. */ - object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE; + object.last_error = OlmErrorCode::OLM_PICKLE_EXTRA_DATA; return std::size_t(-1); } @@ -338,7 +338,7 @@ size_t olm_unpickle_session( return std::size_t(-1); } else if (pos != end) { /* Input was longer than expected. */ - object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE; + object.last_error = OlmErrorCode::OLM_PICKLE_EXTRA_DATA; return std::size_t(-1); } diff --git a/src/outbound_group_session.c b/src/outbound_group_session.c index bc38a5a..cbbba9c 100644 --- a/src/outbound_group_session.c +++ b/src/outbound_group_session.c @@ -160,7 +160,7 @@ size_t olm_unpickle_outbound_group_session( if (pos != end) { /* Input was longer than expected. */ - session->last_error = OLM_CORRUPTED_PICKLE; + session->last_error = OLM_PICKLE_EXTRA_DATA; return (size_t)-1; } diff --git a/src/pk.cpp b/src/pk.cpp index 2cb40d2..9217c48 100644 --- a/src/pk.cpp +++ b/src/pk.cpp @@ -348,7 +348,7 @@ size_t olm_unpickle_pk_decryption( return std::size_t(-1); } else if (pos != end) { /* Input was longer than expected. */ - object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE; + object.last_error = OlmErrorCode::OLM_PICKLE_EXTRA_DATA; return std::size_t(-1); } diff --git a/tests/test_group_session.cpp b/tests/test_group_session.cpp index 9a1e9a3..251971c 100644 --- a/tests/test_group_session.cpp +++ b/tests/test_group_session.cpp @@ -73,7 +73,7 @@ int main() { "secret_key", 10, junk_pickle.data(), junk_pickle_length )); - assert_equals(OLM_CORRUPTED_PICKLE, + assert_equals(OLM_PICKLE_EXTRA_DATA, olm_outbound_group_session_last_error_code(session)); } @@ -128,7 +128,7 @@ int main() { "secret_key", 10, junk_pickle.data(), junk_pickle_length )); - assert_equals(OLM_CORRUPTED_PICKLE, + assert_equals(OLM_PICKLE_EXTRA_DATA, olm_inbound_group_session_last_error_code(session)); } diff --git a/tests/test_olm.cpp b/tests/test_olm.cpp index 0d93c90..6a11fdd 100644 --- a/tests/test_olm.cpp +++ b/tests/test_olm.cpp @@ -83,7 +83,7 @@ const size_t junk_pickle_length = add_junk_suffix_to_pickle( assert_equals(std::size_t(-1), ::olm_unpickle_account(account, "secret_key", 10, junk_pickle.data(), junk_pickle_length)); -assert_equals(OLM_CORRUPTED_PICKLE, olm_account_last_error_code(account)); +assert_equals(OLM_PICKLE_EXTRA_DATA, olm_account_last_error_code(account)); } @@ -174,7 +174,7 @@ const size_t junk_pickle_length = add_junk_suffix_to_pickle( assert_equals(std::size_t(-1), ::olm_unpickle_session(session, "secret_key", 10, junk_pickle.data(), junk_pickle_length)); -assert_equals(OLM_CORRUPTED_PICKLE, olm_session_last_error_code(session)); +assert_equals(OLM_PICKLE_EXTRA_DATA, olm_session_last_error_code(session)); } { /** Loopback test */ diff --git a/tests/test_pk.cpp b/tests/test_pk.cpp index 2db4eae..f975029 100644 --- a/tests/test_pk.cpp +++ b/tests/test_pk.cpp @@ -167,7 +167,7 @@ assert_equals(std::size_t(-1), junk_pickle.data(), junk_pickle_length, pubkey.data(), pubkey.size() )); -assert_equals(OLM_CORRUPTED_PICKLE, olm_pk_decryption_last_error_code(decryption)); +assert_equals(OLM_PICKLE_EXTRA_DATA, olm_pk_decryption_last_error_code(decryption)); /***/ char *ciphertext = strdup("ntk49j/KozVFtSqJXhCejg");