From 1ff64391edf9f2e3180238271858698a5a6f30c6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 15:03:40 +0100 Subject: Fix a buffer bounds check when decoding group messages Fixes a segfault when a group message had exactly the length of the mac + signature. Also tweak skipping of unknown tags to avoid an extra trip around the loop. --- src/message.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/message.cpp b/src/message.cpp index 05fe2c7..1c11a4a 100644 --- a/src/message.cpp +++ b/src/message.cpp @@ -214,11 +214,13 @@ void olm::decode_message( reader.ciphertext = nullptr; reader.ciphertext_length = 0; - if (pos == end) return; if (input_length < mac_length) return; + + if (pos == end) return; reader.version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, RATCHET_KEY_TAG, reader.ratchet_key, reader.ratchet_key_length @@ -234,7 +236,6 @@ void olm::decode_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } } @@ -303,6 +304,7 @@ void olm::decode_one_time_key_message( reader.version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, ONE_TIME_KEY_ID_TAG, reader.one_time_key, reader.one_time_key_length @@ -322,7 +324,6 @@ void olm::decode_one_time_key_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } } @@ -377,9 +378,12 @@ void _olm_decode_group_message( results->ciphertext_length = 0; if (input_length < trailer_length) return; + + if (pos == end) return; results->version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, GROUP_MESSAGE_INDEX_TAG, results->message_index, has_message_index @@ -391,7 +395,6 @@ void _olm_decode_group_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } results->has_message_index = (int)has_message_index; -- cgit v1.2.3 From 653790eacbf7dcf94cbf181657cdb0c30c890c3f Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 20 Oct 2016 09:58:55 +0100 Subject: Return the message index when decrypting group messages. Applications can use the index to detect replays of the same message. --- src/inbound_group_session.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c index bf00008..ed313a0 100644 --- a/src/inbound_group_session.c +++ b/src/inbound_group_session.c @@ -263,7 +263,8 @@ size_t olm_group_decrypt_max_plaintext_length( static size_t _decrypt( OlmInboundGroupSession *session, uint8_t * message, size_t message_length, - uint8_t * plaintext, size_t max_plaintext_length + uint8_t * plaintext, size_t max_plaintext_length, + uint32_t * message_index ) { struct _OlmDecodeGroupMessageResults decoded_results; size_t max_length, r; @@ -286,6 +287,8 @@ static size_t _decrypt( return (size_t)-1; } + *message_index = decoded_results.message_index; + /* verify the signature. We could do this before decoding the message, but * we allow for the possibility of future protocol versions which use a * different signing mechanism; we would rather throw "BAD_MESSAGE_VERSION" @@ -349,7 +352,8 @@ static size_t _decrypt( size_t olm_group_decrypt( OlmInboundGroupSession *session, uint8_t * message, size_t message_length, - uint8_t * plaintext, size_t max_plaintext_length + uint8_t * plaintext, size_t max_plaintext_length, + uint32_t * message_index ) { size_t raw_message_length; @@ -361,7 +365,8 @@ size_t olm_group_decrypt( return _decrypt( session, message, raw_message_length, - plaintext, max_plaintext_length + plaintext, max_plaintext_length, + message_index ); } -- cgit v1.2.3 From 3091dc2b1d5eab575bfe886bf1d6f2414797373f Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 20 Oct 2016 11:35:45 +0100 Subject: Add NULL check for message_index pointer --- src/inbound_group_session.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c index ed313a0..a54e55f 100644 --- a/src/inbound_group_session.c +++ b/src/inbound_group_session.c @@ -287,7 +287,9 @@ static size_t _decrypt( return (size_t)-1; } - *message_index = decoded_results.message_index; + if (message_index != NULL) { + *message_index = decoded_results.message_index; + } /* verify the signature. We could do this before decoding the message, but * we allow for the possibility of future protocol versions which use a -- cgit v1.2.3 From 21ce3491dd39485eac35ad850257a20fc99f330d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Oct 2016 17:19:59 +0100 Subject: Clear random buf in olm_init_outbound_group_session All the other methods clear their random inputs. This one needs to do the same, to reduce the risk of the randomness being used elsewhere and leaking key info. --- src/outbound_group_session.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/outbound_group_session.c b/src/outbound_group_session.c index 4e4561a..ae45694 100644 --- a/src/outbound_group_session.c +++ b/src/outbound_group_session.c @@ -154,20 +154,23 @@ size_t olm_init_outbound_group_session_random_length( size_t olm_init_outbound_group_session( OlmOutboundGroupSession *session, - uint8_t const * random, size_t random_length + uint8_t *random, size_t random_length ) { + const uint8_t *random_ptr = random; + if (random_length < olm_init_outbound_group_session_random_length(session)) { /* Insufficient random data for new session */ session->last_error = OLM_NOT_ENOUGH_RANDOM; return (size_t)-1; } - megolm_init(&(session->ratchet), random, 0); - random += MEGOLM_RATCHET_LENGTH; + megolm_init(&(session->ratchet), random_ptr, 0); + random_ptr += MEGOLM_RATCHET_LENGTH; - _olm_crypto_ed25519_generate_key(random, &(session->signing_key)); - random += ED25519_RANDOM_LENGTH; + _olm_crypto_ed25519_generate_key(random_ptr, &(session->signing_key)); + random_ptr += ED25519_RANDOM_LENGTH; + _olm_unset(random, random_length); return 0; } -- cgit v1.2.3 From a7310c5821513d5c5b0609ec506dad1ae51603d3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 10:06:06 +0100 Subject: Return the base64-encoded length of pickles make olm_pickle_* return the lengths of the base64-encoded pickles, rather than the raw pickle. (From the application's POV, the format of the pickle is opaque: it doesn't even know that it is base64-encoded. So returning the length of the raw pickle is particularly unhelpful.) --- src/pickle_encoding.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/pickle_encoding.c b/src/pickle_encoding.c index 5d5f8d7..a56e9e3 100644 --- a/src/pickle_encoding.c +++ b/src/pickle_encoding.c @@ -60,7 +60,7 @@ size_t _olm_enc_output( raw_output, length ); _olm_encode_base64(raw_output, length, output); - return raw_length; + return base64_length; } -- cgit v1.2.3