From 76610c0a3af57b600211ea38bc28bcccabc6a86c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 14 Dec 2016 11:43:00 +0000 Subject: [PATCH 1/3] Allocate memory for message blobs on the heap Messages can be very large, so we don't really want to allocate them on the stack. Switch to using the heap for them, and try to clean up some of the string handling while we're at it. --- javascript/olm_inbound_group_session.js | 66 +++++++----- javascript/olm_outbound_group_session.js | 44 +++++--- javascript/olm_post.js | 131 ++++++++++++++--------- 3 files changed, 153 insertions(+), 88 deletions(-) diff --git a/javascript/olm_inbound_group_session.js b/javascript/olm_inbound_group_session.js index 1b7fcfe..5815320 100644 --- a/javascript/olm_inbound_group_session.js +++ b/javascript/olm_inbound_group_session.js @@ -64,33 +64,49 @@ InboundGroupSession.prototype['create'] = restore_stack(function(session_key) { InboundGroupSession.prototype['decrypt'] = restore_stack(function( message ) { - var message_array = array_from_string(message); - var message_buffer = stack(message_array); - var max_plaintext_length = inbound_group_session_method( - Module['_olm_group_decrypt_max_plaintext_length'] - )(this.ptr, message_buffer, message_array.length); - // caculating the length destroys the input buffer. - // So we copy the array to a new buffer - var message_buffer = stack(message_array); - var plaintext_buffer = stack(max_plaintext_length + NULL_BYTE_PADDING_LENGTH); - var message_index = stack(4); - var plaintext_length = inbound_group_session_method(Module["_olm_group_decrypt"])( - this.ptr, - message_buffer, message_array.length, - plaintext_buffer, max_plaintext_length, - message_index - ); + var message_buffer, plaintext_buffer; - // Pointer_stringify requires a null-terminated argument (the optional - // 'len' argument doesn't work for UTF-8 data). - Module['setValue']( - plaintext_buffer+plaintext_length, - 0, "i8" - ); + try { + message_buffer = malloc(message.length); + Module['writeAsciiToMemory'](message, message_buffer, true); - return { - "plaintext": Pointer_stringify(plaintext_buffer), - "message_index": Module['getValue'](message_index, "i32") + var max_plaintext_length = inbound_group_session_method( + Module['_olm_group_decrypt_max_plaintext_length'] + )(this.ptr, message_buffer, message.length); + + // caculating the length destroys the input buffer, so we need to re-copy it. + Module['writeAsciiToMemory'](message, message_buffer, true); + + plaintext_buffer = malloc(max_plaintext_length + NULL_BYTE_PADDING_LENGTH); + var message_index = stack(4); + + var plaintext_length = inbound_group_session_method( + Module["_olm_group_decrypt"] + )( + this.ptr, + message_buffer, message.length, + plaintext_buffer, max_plaintext_length, + message_index + ); + + // UTF8ToString requires a null-terminated argument, so add the + // null terminator. + Module['setValue']( + plaintext_buffer+plaintext_length, + 0, "i8" + ); + + return { + "plaintext": UTF8ToString(plaintext_buffer), + "message_index": Module['getValue'](message_index, "i32") + } + } finally { + if (message_buffer !== undefined) { + free(message_buffer); + } + if (plaintext_buffer !== undefined) { + free(plaintext_buffer); + } } }); diff --git a/javascript/olm_outbound_group_session.js b/javascript/olm_outbound_group_session.js index 88a441d..01fee0b 100644 --- a/javascript/olm_outbound_group_session.js +++ b/javascript/olm_outbound_group_session.js @@ -63,20 +63,36 @@ OutboundGroupSession.prototype['create'] = restore_stack(function() { ); }); -OutboundGroupSession.prototype['encrypt'] = restore_stack(function(plaintext) { - var plaintext_array = array_from_string(plaintext); - var message_length = outbound_group_session_method( - Module['_olm_group_encrypt_message_length'] - )(this.ptr, plaintext_array.length); - var plaintext_buffer = stack(plaintext_array); - var message_buffer = stack(message_length + NULL_BYTE_PADDING_LENGTH); - outbound_group_session_method(Module['_olm_group_encrypt'])( - this.ptr, - plaintext_buffer, plaintext_array.length, - message_buffer, message_length - ); - return Pointer_stringify(message_buffer); -}); +OutboundGroupSession.prototype['encrypt'] = function(plaintext) { + var plaintext_buffer, message_buffer; + try { + var plaintext_length = Module['lengthBytesUTF8'](plaintext); + + var message_length = outbound_group_session_method( + Module['_olm_group_encrypt_message_length'] + )(this.ptr, plaintext_length); + + // need to allow space for the terminator (which stringToUTF8 always + // writes), hence + 1. + plaintext_buffer = malloc(plaintext_length + 1); + Module['stringToUTF8'](plaintext, plaintext_buffer, plaintext_length + 1); + + message_buffer = malloc(message_length + NULL_BYTE_PADDING_LENGTH); + outbound_group_session_method(Module['_olm_group_encrypt'])( + this.ptr, + plaintext_buffer, plaintext_length, + message_buffer, message_length + ); + return Module['UTF8ToString'](message_buffer); + } finally { + if (plaintext_buffer !== undefined) { + free(plaintext_buffer); + } + if (message_buffer !== undefined) { + free(message_buffer); + } + } +}; OutboundGroupSession.prototype['session_id'] = restore_stack(function() { var length = outbound_group_session_method( diff --git a/javascript/olm_post.js b/javascript/olm_post.js index 8951c11..9820354 100644 --- a/javascript/olm_post.js +++ b/javascript/olm_post.js @@ -4,9 +4,11 @@ var free = Module['_free']; var Pointer_stringify = Module['Pointer_stringify']; var OLM_ERROR = Module['_olm_error'](); -/* The 'length' argument to Pointer_stringify doesn't work if the input includes - * characters >= 128; we therefore need to add a NULL character to all of our - * strings. This acts as a symbolic constant to help show what we're doing. +/* The 'length' argument to Pointer_stringify doesn't work if the input + * includes characters >= 128, which makes Pointer_stringify unreliable. We + * could use it on strings which are known to be ascii, but that seems + * dangerous. Instead we add a NULL character to all of our strings and just + * use UTF8ToString. */ var NULL_BYTE_PADDING_LENGTH = 1; @@ -297,59 +299,90 @@ Session.prototype['matches_inbound_from'] = restore_stack(function( Session.prototype['encrypt'] = restore_stack(function( plaintext ) { - var random_length = session_method( - Module['_olm_encrypt_random_length'] - )(this.ptr); - var message_type = session_method( - Module['_olm_encrypt_message_type'] - )(this.ptr); - var plaintext_array = array_from_string(plaintext); - var message_length = session_method( - Module['_olm_encrypt_message_length'] - )(this.ptr, plaintext_array.length); - var random = random_stack(random_length); - var plaintext_buffer = stack(plaintext_array); - var message_buffer = stack(message_length + NULL_BYTE_PADDING_LENGTH); - session_method(Module['_olm_encrypt'])( - this.ptr, - plaintext_buffer, plaintext_array.length, - random, random_length, - message_buffer, message_length - ); - return { - "type": message_type, - "body": Pointer_stringify(message_buffer) - }; + var plaintext_buffer, message_buffer; + try { + var random_length = session_method( + Module['_olm_encrypt_random_length'] + )(this.ptr); + var message_type = session_method( + Module['_olm_encrypt_message_type'] + )(this.ptr); + + var plaintext_length = Module['lengthBytesUTF8'](plaintext); + var message_length = session_method( + Module['_olm_encrypt_message_length'] + )(this.ptr, plaintext_length); + + var random = random_stack(random_length); + + // need to allow space for the terminator (which stringToUTF8 always + // writes), hence + 1. + plaintext_buffer = malloc(plaintext_length + 1); + Module['stringToUTF8'](plaintext, plaintext_buffer, plaintext_length + 1); + + message_buffer = malloc(message_length + NULL_BYTE_PADDING_LENGTH); + + session_method(Module['_olm_encrypt'])( + this.ptr, + plaintext_buffer, plaintext_length, + random, random_length, + message_buffer, message_length + ); + return { + "type": message_type, + "body": Module['UTF8ToString'](message_buffer), + }; + } finally { + if (plaintext_buffer !== undefined) { + free(plaintext_buffer); + } + if (message_buffer !== undefined) { + free(message_buffer); + } + } }); Session.prototype['decrypt'] = restore_stack(function( message_type, message ) { - var message_array = array_from_string(message); - var message_buffer = stack(message_array); - var max_plaintext_length = session_method( - Module['_olm_decrypt_max_plaintext_length'] - )(this.ptr, message_type, message_buffer, message_array.length); - // caculating the length destroys the input buffer. - // So we copy the array to a new buffer - var message_buffer = stack(message_array); - var plaintext_buffer = stack( - max_plaintext_length + NULL_BYTE_PADDING_LENGTH - ); - var plaintext_length = session_method(Module["_olm_decrypt"])( - this.ptr, message_type, - message_buffer, message.length, - plaintext_buffer, max_plaintext_length - ); + var message_buffer, plaintext_buffer; - // Pointer_stringify requires a null-terminated argument (the optional - // 'len' argument doesn't work for UTF-8 data). - Module['setValue']( - plaintext_buffer+plaintext_length, - 0, "i8" - ); + try { + message_buffer = malloc(message.length); + Module['writeAsciiToMemory'](message, message_buffer, true); + + var max_plaintext_length = session_method( + Module['_olm_decrypt_max_plaintext_length'] + )(this.ptr, message_type, message_buffer, message.length); + + // caculating the length destroys the input buffer, so we need to re-copy it. + Module['writeAsciiToMemory'](message, message_buffer, true); + + plaintext_buffer = malloc(max_plaintext_length + NULL_BYTE_PADDING_LENGTH); + + var plaintext_length = session_method(Module["_olm_decrypt"])( + this.ptr, message_type, + message_buffer, message.length, + plaintext_buffer, max_plaintext_length + ); + + // UTF8ToString requires a null-terminated argument, so add the + // null terminator. + Module['setValue']( + plaintext_buffer+plaintext_length, + 0, "i8" + ); + + return UTF8ToString(plaintext_buffer); + } finally { + if (message_buffer !== undefined) { + free(message_buffer); + } + if (plaintext_buffer !== undefined) { + free(plaintext_buffer); + } + } - return Pointer_stringify(plaintext_buffer); }); function Utility() { From 8356fa37adbe1662141f93cc749e4c2d05af9f7b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 15 Dec 2016 13:37:34 +0000 Subject: [PATCH 2/3] zero out plaintext buffers Avoid leaving copies of the plaintext sitting around in the emscripten heap. --- javascript/olm_inbound_group_session.js | 6 ++++-- javascript/olm_outbound_group_session.js | 6 ++++-- javascript/olm_post.js | 19 +++++++++++++++---- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/javascript/olm_inbound_group_session.js b/javascript/olm_inbound_group_session.js index 5815320..2e4727f 100644 --- a/javascript/olm_inbound_group_session.js +++ b/javascript/olm_inbound_group_session.js @@ -64,7 +64,7 @@ InboundGroupSession.prototype['create'] = restore_stack(function(session_key) { InboundGroupSession.prototype['decrypt'] = restore_stack(function( message ) { - var message_buffer, plaintext_buffer; + var message_buffer, plaintext_buffer, plaintext_length; try { message_buffer = malloc(message.length); @@ -80,7 +80,7 @@ InboundGroupSession.prototype['decrypt'] = restore_stack(function( plaintext_buffer = malloc(max_plaintext_length + NULL_BYTE_PADDING_LENGTH); var message_index = stack(4); - var plaintext_length = inbound_group_session_method( + plaintext_length = inbound_group_session_method( Module["_olm_group_decrypt"] )( this.ptr, @@ -105,6 +105,8 @@ InboundGroupSession.prototype['decrypt'] = restore_stack(function( free(message_buffer); } if (plaintext_buffer !== undefined) { + // don't leave a copy of the plaintext in the heap. + bzero(plaintext_buffer, plaintext_length + NULL_BYTE_PADDING_LENGTH); free(plaintext_buffer); } } diff --git a/javascript/olm_outbound_group_session.js b/javascript/olm_outbound_group_session.js index 01fee0b..0402c3c 100644 --- a/javascript/olm_outbound_group_session.js +++ b/javascript/olm_outbound_group_session.js @@ -64,9 +64,9 @@ OutboundGroupSession.prototype['create'] = restore_stack(function() { }); OutboundGroupSession.prototype['encrypt'] = function(plaintext) { - var plaintext_buffer, message_buffer; + var plaintext_buffer, message_buffer, plaintext_length; try { - var plaintext_length = Module['lengthBytesUTF8'](plaintext); + plaintext_length = Module['lengthBytesUTF8'](plaintext); var message_length = outbound_group_session_method( Module['_olm_group_encrypt_message_length'] @@ -86,6 +86,8 @@ OutboundGroupSession.prototype['encrypt'] = function(plaintext) { return Module['UTF8ToString'](message_buffer); } finally { if (plaintext_buffer !== undefined) { + // don't leave a copy of the plaintext in the heap. + bzero(plaintext_buffer, plaintext_length + 1); free(plaintext_buffer); } if (message_buffer !== undefined) { diff --git a/javascript/olm_post.js b/javascript/olm_post.js index 9820354..752279a 100644 --- a/javascript/olm_post.js +++ b/javascript/olm_post.js @@ -42,6 +42,13 @@ function restore_stack(wrapped) { } } +/* set a memory area to zero */ +function bzero(ptr, n) { + while(n-- > 0) { + Module['HEAP8'][ptr++] = 0; + } +} + function Account() { var size = Module['_olm_account_size'](); this.buf = malloc(size); @@ -299,7 +306,7 @@ Session.prototype['matches_inbound_from'] = restore_stack(function( Session.prototype['encrypt'] = restore_stack(function( plaintext ) { - var plaintext_buffer, message_buffer; + var plaintext_buffer, message_buffer, plaintext_length; try { var random_length = session_method( Module['_olm_encrypt_random_length'] @@ -308,7 +315,7 @@ Session.prototype['encrypt'] = restore_stack(function( Module['_olm_encrypt_message_type'] )(this.ptr); - var plaintext_length = Module['lengthBytesUTF8'](plaintext); + plaintext_length = Module['lengthBytesUTF8'](plaintext); var message_length = session_method( Module['_olm_encrypt_message_length'] )(this.ptr, plaintext_length); @@ -334,6 +341,8 @@ Session.prototype['encrypt'] = restore_stack(function( }; } finally { if (plaintext_buffer !== undefined) { + // don't leave a copy of the plaintext in the heap. + bzero(plaintext_buffer, plaintext_length + 1); free(plaintext_buffer); } if (message_buffer !== undefined) { @@ -345,13 +354,13 @@ Session.prototype['encrypt'] = restore_stack(function( Session.prototype['decrypt'] = restore_stack(function( message_type, message ) { - var message_buffer, plaintext_buffer; + var message_buffer, plaintext_buffer, max_pliantext_length; try { message_buffer = malloc(message.length); Module['writeAsciiToMemory'](message, message_buffer, true); - var max_plaintext_length = session_method( + max_plaintext_length = session_method( Module['_olm_decrypt_max_plaintext_length'] )(this.ptr, message_type, message_buffer, message.length); @@ -379,6 +388,8 @@ Session.prototype['decrypt'] = restore_stack(function( free(message_buffer); } if (plaintext_buffer !== undefined) { + // don't leave a copy of the plaintext in the heap. + bzero(plaintext_buffer, max_plaintext_length + NULL_BYTE_PADDING_LENGTH); free(plaintext_buffer); } } From 09b3e1eecdd83c738a1bcd1aca7319206eaf7091 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 15 Dec 2016 16:28:30 +0000 Subject: [PATCH 3/3] typo --- javascript/olm_post.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/olm_post.js b/javascript/olm_post.js index 752279a..fd74122 100644 --- a/javascript/olm_post.js +++ b/javascript/olm_post.js @@ -354,7 +354,7 @@ Session.prototype['encrypt'] = restore_stack(function( Session.prototype['decrypt'] = restore_stack(function( message_type, message ) { - var message_buffer, plaintext_buffer, max_pliantext_length; + var message_buffer, plaintext_buffer, max_plaintext_length; try { message_buffer = malloc(message.length);