From 9904f4c715f5e15da7967eb75715354e4a4b7a32 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Sun, 5 Jul 2020 23:20:21 -0400 Subject: [PATCH] Try to detect if cipherText is plainText. In some situations, the cipherText that was passed in is actually plainText. Old mpsites files used to store the login name as plain text even though the file was redacted. Newer versions of the file store the login name as ciphertext. There is no clear way to distinguish between the two cases. --- platform-independent/c/core/src/mpw-algorithm_v0.c | 9 ++++++++- platform-independent/c/core/src/mpw-marshal.c | 8 +++----- platform-independent/c/core/src/mpw-util.c | 8 ++++++-- platform-independent/c/core/src/mpw-util.h | 4 ++-- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/platform-independent/c/core/src/mpw-algorithm_v0.c b/platform-independent/c/core/src/mpw-algorithm_v0.c index c2db3e45..1d056dd6 100644 --- a/platform-independent/c/core/src/mpw-algorithm_v0.c +++ b/platform-independent/c/core/src/mpw-algorithm_v0.c @@ -166,6 +166,11 @@ const char *mpw_site_crypted_password_v0( err( "Missing encrypted state." ); return NULL; } + if (strlen( cipherText ) % 4 != 0) { + wrn( "Malformed encrypted state, not base64." ); + // This can happen if state was stored in a non-encrypted form, eg. login in old mpsites. + return strdup( cipherText ); + } // Base64-decode uint8_t *cipherBuf = calloc( 1, mpw_base64_decode_max( cipherText ) ); @@ -184,8 +189,10 @@ const char *mpw_site_crypted_password_v0( mpw_free( &plainBytes, bufSize ); if (!plainText) err( "AES decryption error: %s", strerror( errno ) ); + else if (!mpw_utf8_strchars( plainText )) + wrn( "decrypted -> plainText: %zu bytes = illegal UTF-8 = %s", strlen( plainText ), mpw_hex( plainText, bufSize ) ); else - trc( "decrypted -> plainText: %zu bytes = %s = %s", strlen( plainText ), plainText, mpw_hex( plainText, strlen( plainText ) ) ); + trc( "decrypted -> plainText: %zu bytes = %s = %s", strlen( plainText ), plainText, mpw_hex( plainText, bufSize ) ); return plainText; } diff --git a/platform-independent/c/core/src/mpw-marshal.c b/platform-independent/c/core/src/mpw-marshal.c index 80b4469b..51cab03b 100644 --- a/platform-independent/c/core/src/mpw-marshal.c +++ b/platform-independent/c/core/src/mpw-marshal.c @@ -980,7 +980,7 @@ static void mpw_marshal_read_flat( str_counter = mpw_strdup( strtok( NULL, "" ) ); mpw_free_string( &typeAndVersionAndCounter ); } - siteLoginState = mpw_get_token( &positionInLine, endOfLine, "\t\n" ); // TODO: Needs to be encoded if redacted? + siteLoginState = mpw_get_token( &positionInLine, endOfLine, "\t\n" ); siteName = mpw_get_token( &positionInLine, endOfLine, "\t\n" ); siteResultState = mpw_get_token( &positionInLine, endOfLine, "\n" ); break; @@ -1019,11 +1019,9 @@ static void mpw_marshal_read_flat( mpw_marshal_data_set_num( siteCounter, file->data, "sites", siteName, "counter", NULL ); mpw_marshal_data_set_num( siteAlgorithm, file->data, "sites", siteName, "algorithm", NULL ); mpw_marshal_data_set_num( siteType, file->data, "sites", siteName, "type", NULL ); - mpw_marshal_data_set_str( siteResultState && strlen( siteResultState )? siteResultState: NULL, file->data, "sites", siteName, - "password", NULL ); + mpw_marshal_data_set_str( siteResultState, file->data, "sites", siteName, "password", NULL ); mpw_marshal_data_set_num( MPResultTypeDefault, file->data, "sites", siteName, "login_type", NULL ); - mpw_marshal_data_set_str( siteLoginState && strlen( siteLoginState )? siteLoginState: NULL, file->data, "sites", siteName, - "login_name", NULL ); + mpw_marshal_data_set_str( siteLoginState, file->data, "sites", siteName, "login_name", NULL ); mpw_marshal_data_set_num( strtol( str_uses, NULL, 10 ), file->data, "sites", siteName, "uses", NULL ); if (strftime( dateString, sizeof( dateString ), "%FT%TZ", gmtime( &siteLastUsed ) )) mpw_marshal_data_set_str( dateString, file->data, "sites", siteName, "last_used", NULL ); diff --git a/platform-independent/c/core/src/mpw-util.c b/platform-independent/c/core/src/mpw-util.c index 9c024747..4d265cbe 100644 --- a/platform-independent/c/core/src/mpw-util.c +++ b/platform-independent/c/core/src/mpw-util.c @@ -607,6 +607,9 @@ const uint8_t *mpw_unhex(const char *hex, size_t *length) { size_t mpw_utf8_charlen(const char *utf8String) { + if (!utf8String) + return 0; + // Legal UTF-8 byte sequences: unsigned char utf8Char = (unsigned char)*utf8String; if (utf8Char >= 0x00 && utf8Char <= 0x7F) @@ -624,8 +627,9 @@ size_t mpw_utf8_charlen(const char *utf8String) { size_t mpw_utf8_strchars(const char *utf8String) { size_t strchars = 0, charlen; - for (char *remaining = (char *)utf8String; (charlen = mpw_utf8_charlen( remaining )); remaining += charlen) - ++strchars; + for (char *remaining = (char *)utf8String; remaining && *remaining; remaining += charlen, ++strchars) + if (!(charlen = mpw_utf8_charlen( remaining ))) + return 0; return strchars; } diff --git a/platform-independent/c/core/src/mpw-util.h b/platform-independent/c/core/src/mpw-util.h index ec18dde5..2e9fd23b 100644 --- a/platform-independent/c/core/src/mpw-util.h +++ b/platform-independent/c/core/src/mpw-util.h @@ -256,9 +256,9 @@ bool mpw_id_buf_equals(MPKeyID id1, MPKeyID id2); //// String utilities. -/** @return The byte length of the UTF-8 character at the start of the given string. */ +/** @return The byte length of the UTF-8 character at the start of the given string or 0 if it is NULL, empty or not a legal UTF-8 character. */ size_t mpw_utf8_charlen(const char *utf8String); -/** @return The amount of UTF-8 characters in the given string. */ +/** @return The amount of UTF-8 characters in the given string or 0 if it is NULL, empty, or contains bytes that are not legal in UTF-8. */ size_t mpw_utf8_strchars(const char *utf8String); /** Drop-in for memdup(3). * @return A buffer (allocated, len) with len bytes copied from src or NULL if src is missing or the buffer could not be allocated. */