diff --git a/app/src/androidTest/java/es/wolfi/app/passman/offline/NativeCryptoMemoryTest.java b/app/src/androidTest/java/es/wolfi/app/passman/offline/NativeCryptoMemoryTest.java index d199d09f..bd1f8d62 100644 --- a/app/src/androidTest/java/es/wolfi/app/passman/offline/NativeCryptoMemoryTest.java +++ b/app/src/androidTest/java/es/wolfi/app/passman/offline/NativeCryptoMemoryTest.java @@ -21,16 +21,21 @@ package es.wolfi.app.passman.offline; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import android.os.Debug; +import android.util.Base64; import android.util.Log; import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.json.JSONObject; import org.junit.Test; import org.junit.runner.RunWith; +import java.nio.charset.StandardCharsets; + import es.wolfi.app.passman.SJCLCrypto; /** @@ -82,6 +87,26 @@ public void encryptDoesNotLeakNativeMemory() throws Exception { assertGrowthBelowLimit("encrypt", before, Debug.getNativeHeapAllocatedSize()); } + @Test + public void rejectedDecryptionDoesNotLeakNativeMemory() throws Exception { + JSONObject parameters = new JSONObject(new String( + Base64.decode(SJCLCrypto.encryptString(PLAINTEXT, PASSWORD, true), Base64.DEFAULT), + StandardCharsets.UTF_8 + )); + // the large ct and the salt are decoded before the undecodable iv lets the decryption fail + parameters.put("iv", "A="); + String rejected = Base64.encodeToString(parameters.toString().getBytes(StandardCharsets.UTF_8), Base64.NO_WRAP); + for (int i = 0; i < 20; i++) { + assertThrows(Exception.class, () -> SJCLCrypto.decryptStringCpp(rejected, PASSWORD)); + } + + long before = Debug.getNativeHeapAllocatedSize(); + for (int i = 0; i < RUNS; i++) { + assertThrows(Exception.class, () -> SJCLCrypto.decryptStringCpp(rejected, PASSWORD)); + } + assertGrowthBelowLimit("rejected decrypt", before, Debug.getNativeHeapAllocatedSize()); + } + private static void assertGrowthBelowLimit(String operation, long before, long after) { long growth = after - before; Log.i(TAG, operation + ": native heap grew by " + growth + " bytes in " + RUNS + " runs"); diff --git a/app/src/androidTest/java/es/wolfi/app/passman/offline/SjclInputValidationTest.java b/app/src/androidTest/java/es/wolfi/app/passman/offline/SjclInputValidationTest.java new file mode 100644 index 00000000..9be27695 --- /dev/null +++ b/app/src/androidTest/java/es/wolfi/app/passman/offline/SjclInputValidationTest.java @@ -0,0 +1,204 @@ +/** + * Passman Android App + * + * @copyright Copyright (c) 2026, Timo Triebensky (timo@binsky.org) + * @license GNU AGPL version 3 or any later version + *

+ * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + *

+ * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + *

+ * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package es.wolfi.app.passman.offline; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import android.util.Base64; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.json.JSONObject; +import org.junit.Before; +import org.junit.Test; +import org.junit.function.ThrowingRunnable; +import org.junit.runner.RunWith; + +import java.nio.charset.StandardCharsets; + +import es.wolfi.app.passman.SJCLCrypto; + +/** + * Cryptograms come from the server or the offline cache, so the native SJCL implementation has to reject + * malformed ones with an exception instead of crashing the app. The same applies to a failed encryption. + */ +@RunWith(AndroidJUnit4.class) +public class SjclInputValidationTest { + private static final String PASSWORD = "vault password"; + private static final String PLAINTEXT = "secret"; + private static final String[] STRING_FIELDS = {"iv", "adata", "salt", "ct"}; + private static final String[] NUMBER_FIELDS = {"iter", "ks", "ts"}; + + private JSONObject parameters; + + @Before + public void encrypt() throws Exception { + parameters = new JSONObject(new String( + Base64.decode(SJCLCrypto.encryptString(PLAINTEXT, PASSWORD, true), Base64.DEFAULT), + StandardCharsets.UTF_8 + )); + } + + @Test + public void unmodifiedCryptogramIsDecrypted() throws Exception { + assertEquals(PLAINTEXT, SJCLCrypto.decryptString(cryptogram(parameters.toString()), PASSWORD)); + } + + @Test + public void wrongPasswordThrowsTheDecryptionError() { + // the exception the app gets from the Java wrapper + assertError("Error decrypting", () -> SJCLCrypto.decryptString(cryptogram(parameters.toString()), PASSWORD + "x")); + } + + @Test + public void noJsonObjectIsRejected() { + for (String json : new String[]{"no json", "[]", "\"text\"", "42"}) { + assertRejected(json, json); + } + } + + @Test + public void missingFieldIsRejected() throws Exception { + for (String field : concat(STRING_FIELDS, NUMBER_FIELDS)) { + JSONObject manipulated = new JSONObject(parameters.toString()); + manipulated.remove(field); + assertRejected("without " + field, manipulated.toString()); + } + } + + @Test + public void stringFieldOfWrongTypeIsRejected() throws Exception { + for (String field : STRING_FIELDS) { + for (Object value : new Object[]{1, true, JSONObject.NULL, new JSONObject()}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put(field, value); + assertRejected(field + ": " + value, manipulated.toString()); + } + } + } + + @Test + public void numberFieldOfWrongTypeOrRangeIsRejected() throws Exception { + for (String field : NUMBER_FIELDS) { + for (Object value : new Object[]{"1000", true, JSONObject.NULL, 0, -1, 1.5, 1e300, -1e300, 2147483648L}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put(field, value); + assertRejected(field + ": " + value, manipulated.toString()); + } + } + } + + @Test + public void unsupportedKeySizeIsRejected() throws Exception { + // the web client and the app only use AES-256 + for (int keySize : new int[]{128, 192, 1, 512, Integer.MAX_VALUE}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put("ks", keySize); + assertRejected("ks: " + keySize, manipulated.toString()); + } + } + + @Test + public void unsupportedTagSizeIsRejected() throws Exception { + // the web client and the app only use 64 bit tags + for (int tagSize : new int[]{32, 96, 128, 8, 1, Integer.MAX_VALUE}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put("ts", tagSize); + assertRejected("ts: " + tagSize, manipulated.toString()); + } + } + + @Test + public void cryptogramShorterThanTheTagIsRejected() throws Exception { + // the 64 bit tag takes 8 bytes + for (int length : new int[]{0, 1, 4, 7}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put("ct", base64(new byte[length])); + assertRejected("ct of " + length + " bytes", manipulated.toString()); + } + } + + @Test + public void tooShortIvIsRejected() throws Exception { + // the nonce of a cryptogram below 64 KB takes 13 bytes + for (int length : new int[]{0, 1, 8, 12}) { + JSONObject manipulated = new JSONObject(parameters.toString()).put("iv", base64(new byte[length])); + assertRejected("iv of " + length + " bytes", manipulated.toString()); + } + } + + @Test + public void invalidBase64IsRejected() throws Exception { + // a single character before the padding can't be decoded, + // unlike characters outside of the base64 alphabet, which the native base64 decoding skips + String[] invalid = {"A=", "AAAAA=="}; + for (String value : invalid) { + assertThrows(value, Exception.class, () -> SJCLCrypto.decryptStringCpp(value, PASSWORD)); + } + for (String field : new String[]{"ct", "salt", "iv"}) { + for (String value : invalid) { + JSONObject manipulated = new JSONObject(parameters.toString()).put(field, value); + assertRejected(field + ": " + value, manipulated.toString()); + } + } + } + + /** + * (it was a surprise it did not work before this fix commit) + */ + @Test + public void failedEncryptionThrowsAnException() { + // the native encryption fails for an empty plaintext, the app avoids this by JSON encoding it + assertError("Error encrypting", () -> SJCLCrypto.encryptStringCpp("", PASSWORD)); + } + + private static void assertRejected(String message, String json) { + assertError(message, "Error decrypting", () -> SJCLCrypto.decryptStringCpp(cryptogram(json), PASSWORD)); + } + + private static void assertError(String expectedMessage, ThrowingRunnable runnable) { + assertError(null, expectedMessage, runnable); + } + + /** + * The native crypto reports every failure with a plain RuntimeException and a fixed message. + */ + private static void assertError(String message, String expectedMessage, ThrowingRunnable runnable) { + Exception exception = assertThrows(message, Exception.class, runnable); + assertEquals(message, RuntimeException.class, exception.getClass()); + assertEquals(message, expectedMessage, exception.getMessage()); + } + + private static String cryptogram(String json) { + return base64(json.getBytes(StandardCharsets.UTF_8)); + } + + private static String base64(byte[] data) { + return Base64.encodeToString(data, Base64.NO_WRAP); + } + + /** + * A new array with the elements of both arrays, e.g. to check all string and number fields in one loop. + * Java has no built-in array concatenation, and Stream.concat would need API 24 (minSdk is 23). + */ + private static String[] concat(String[] first, String[] second) { + String[] result = new String[first.length + second.length]; + System.arraycopy(first, 0, result, 0, first.length); + System.arraycopy(second, 0, result, first.length, second.length); + return result; + } +} diff --git a/app/src/main/cpp/SJCL.cpp b/app/src/main/cpp/SJCL.cpp index 77b0a872..9093c3bd 100644 --- a/app/src/main/cpp/SJCL.cpp +++ b/app/src/main/cpp/SJCL.cpp @@ -31,6 +31,8 @@ #include #include #include +#include +#include #define LOG_TAG "SJCL" @@ -39,8 +41,8 @@ void handleErrors(const char* error){ } int decryptccm(unsigned char *ciphertext, int ciphertext_len, unsigned char *aad, - int aad_len, unsigned char *tag, unsigned char *key, unsigned char *iv, - unsigned char *plaintext + int aad_len, unsigned char *tag, int tag_len, unsigned char *key, unsigned char *iv, + int iv_len, unsigned char *plaintext ) { EVP_CIPHER_CTX *ctx; int len; @@ -51,8 +53,17 @@ int decryptccm(unsigned char *ciphertext, int ciphertext_len, unsigned char *aad if (ciphertext_len >= 1<<16) lol++; if (ciphertext_len >= 1<<24) lol++; + // OpenSSL reads the first 15-lol bytes of the IV as nonce + if (iv_len < 15-lol) { + handleErrors("The IV is too short"); + return -1; + } + /* Create and initialise the context */ - if (!(ctx = EVP_CIPHER_CTX_new())) handleErrors("Error initializing context"); + if (!(ctx = EVP_CIPHER_CTX_new())) { + handleErrors("Error initializing context"); + goto CLEANUP; + } /* Initialise the decryption operation. */ if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_ccm(), nullptr, nullptr, nullptr)) { @@ -66,7 +77,7 @@ int decryptccm(unsigned char *ciphertext, int ciphertext_len, unsigned char *aad } /* Set expected tag value. */ - if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_TAG, 8, tag)) { + if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_TAG, tag_len, tag)) { handleErrors("Error setting TAG value"); goto CLEANUP; } @@ -225,6 +236,9 @@ int encryptccm(unsigned char *plaintext, int plaintext_len, */ char* wstring_to_char(wstring str) { char* c = (char *) malloc(sizeof(char) * str.length() + 1); + if (c == nullptr) { + return nullptr; + } const wchar_t* data = str.c_str(); for (int i = 0; i <= str.length(); i++) { c[i] = (char) data[i]; @@ -232,63 +246,158 @@ char* wstring_to_char(wstring str) { return c; } +/** + * The string value of the key, nullptr if the key is missing or its value is no string. + * Replaces raw access like with food[L"iv"]. + */ +const wstring* getString(const JSONObject& object, const wchar_t* key) { + auto value = object.find(key); + if (value == object.end() || ! value->second->IsString()) { + return nullptr; + } + return &value->second->AsString(); +} + +/** + * The value of the key as positive int, 0 if the key is missing or its value is no such number. + */ +int getPositiveInt(const JSONObject& object, const wchar_t* key) { + auto value = object.find(key); + if (value == object.end() || ! value->second->IsNumber()) { + return 0; + } + double number = value->second->AsNumber(); + // the cast of a number out of the int range would be undefined behavior + if (! (number >= 1 && number <= std::numeric_limits::max()) || number != std::floor(number)) { + return 0; + } + return (int) number; +} + using namespace WLF::Crypto; +void releaseDatagram(Datagram* datagram) { + if (datagram != nullptr) { + free(datagram->data); + delete datagram; + } +} + char* SJCL::decrypt(const string& sjcl_json, const string& key) { JSONValue *data = JSON::Parse(sjcl_json.c_str()); if (data == nullptr || ! data->IsObject()) { __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "Error parsing the SJCL JSON"); + delete data; return nullptr; } - JSONObject food = data->AsObject(); - - int iter = 0, - key_size = 0, - tag_size = 0; - char *iv_64, *salt_64, *cyphertext, *adata; + const JSONObject& food = data->AsObject(); // Extract the requiered values from the JSON - iv_64 = wstring_to_char(food[L"iv"]->AsString()); - adata = wstring_to_char(food[L"adata"]->AsString()); - salt_64 = wstring_to_char(food[L"salt"]->AsString()); - cyphertext = wstring_to_char(food[L"ct"]->AsString()); + const wstring *iv_value = getString(food, L"iv"), + *adata_value = getString(food, L"adata"), + *salt_value = getString(food, L"salt"), + *ct_value = getString(food, L"ct"); + + int iter = getPositiveInt(food, L"iter"), + key_size = getPositiveInt(food, L"ks"), + tag_size = getPositiveInt(food, L"ts"); + + if (iv_value == nullptr || adata_value == nullptr || salt_value == nullptr || ct_value == nullptr + || iter == 0 || key_size == 0 || tag_size == 0) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "The SJCL JSON lacks a required value"); + delete data; + return nullptr; + } + + // The cipher is always AES-256, it would read a smaller key past its end + if (key_size != 256) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "Unsupported key size, only 256 bit keys are supported"); + delete data; + return nullptr; + } - iter = (int) food[L"iter"]->AsNumber(); - key_size = (int) food[L"ks"]->AsNumber(); - tag_size = (int) food[L"ts"]->AsNumber(); + // Passman only uses 64 bit tags, a smaller one would be read past the end of the cryptogram + if (tag_size != 64) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "Unsupported tag size, only 64 bit tags are supported"); + delete data; + return nullptr; + } + + char *iv_64 = wstring_to_char(*iv_value), + *adata = wstring_to_char(*adata_value), + *salt_64 = wstring_to_char(*salt_value), + *cyphertext = wstring_to_char(*ct_value); + + if (iv_64 == nullptr || adata == nullptr || salt_64 == nullptr || cyphertext == nullptr) { + handleErrors("Out of memory"); // ... i guess + free(iv_64); + free(adata); + free(salt_64); + free(cyphertext); + delete data; + return nullptr; + } tag_size = (tag_size +7) / 8; // Make it bytes! key_size = (key_size +7) / 8; // Make it bytes - // The actual cryptogram includes the tag size, so we need to take this into account later on! Datagram* cryptogram = BASE64::decode((unsigned char *) cyphertext, strlen(cyphertext)); - int cyphertext_data_length = cryptogram->length - tag_size; - Datagram* salt = BASE64::decode((unsigned char *) salt_64, strlen(salt_64)); Datagram* iv_raw = BASE64::decode((unsigned char *) iv_64, strlen(iv_64)); // Datagram* aadata = BASE64::decode((const unsigned char *) "", 0); // Not sure if this is required since we don't use adata + if (cryptogram == nullptr || salt == nullptr || iv_raw == nullptr) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "The SJCL JSON contains invalid base64"); + free(iv_64); + free(adata); + free(salt_64); + free(cyphertext); + delete data; + releaseDatagram(cryptogram); + releaseDatagram(salt); + releaseDatagram(iv_raw); + return nullptr; + } + + // The actual cryptogram includes the tag size, so we need to take this into account later on! + int cyphertext_data_length = cryptogram->length - tag_size; + unsigned char* derived_key = (unsigned char*) malloc(sizeof(unsigned char) * key_size); // Assuming plaintext will always be smaller than the sjcl cyphertext which includes the tag and padding and stuff int cryptogram_length = cryptogram->length; unsigned char* plaintext = (unsigned char*) std::calloc(cryptogram_length +1, sizeof(unsigned char)); - string s = string("The allocated string is: ") + string((char*)plaintext); - - /* PBKDF2 Key derivation with SHA256 as SJCL does by default */ - PKCS5_PBKDF2_HMAC(key.c_str(), key.length(), salt->data, salt->length, iter, EVP_sha256(), key_size, derived_key); char* ret = nullptr; - // Decrypt the data - int plaintext_len = decryptccm(cryptogram->data, cyphertext_data_length, (unsigned char *) adata, strlen(adata), - &cryptogram->data[cyphertext_data_length], derived_key, iv_raw->data, plaintext); + int plaintext_len = -1; + if (derived_key != nullptr && plaintext != nullptr) { + // The cryptogram is the ciphertext followed by the tag, so it can't be shorter than the tag + if (0 <= cyphertext_data_length) { + /* PBKDF2 Key derivation with SHA256 as SJCL does by default */ + if (1 == PKCS5_PBKDF2_HMAC(key.c_str(), key.length(), salt->data, salt->length, iter, EVP_sha256(), key_size, derived_key)) { + // Decrypt the data + plaintext_len = decryptccm(cryptogram->data, cyphertext_data_length, (unsigned char *) adata, strlen(adata), + &cryptogram->data[cyphertext_data_length], tag_size, derived_key, iv_raw->data, iv_raw->length, plaintext); + } else { + handleErrors("Error deriving the key"); + } + } else { + handleErrors("The cryptogram is shorter than the tag"); + } + } else { + handleErrors("Out of memory"); + } if (0 < plaintext_len) { Datagram* plaintext_base64 = BASE64::encode(plaintext, plaintext_len); - ret = reinterpret_cast(plaintext_base64->data); - delete plaintext_base64; + if (plaintext_base64 != nullptr) { + ret = reinterpret_cast(plaintext_base64->data); + delete plaintext_base64; + } else { + handleErrors("Error encoding the plaintext"); + } /* do decoding now in the java part * @@ -311,16 +420,17 @@ char* SJCL::decrypt(const string& sjcl_json, const string& key) { free(salt_64); free(cyphertext); delete data; - free(cryptogram->data); - delete cryptogram; - free(salt->data); - delete salt; - free(iv_raw->data); - delete iv_raw; - OPENSSL_cleanse(plaintext, cryptogram_length + 1); - free(plaintext); - OPENSSL_cleanse(derived_key, key_size); - free(derived_key); + releaseDatagram(cryptogram); + releaseDatagram(salt); + releaseDatagram(iv_raw); + if (plaintext != nullptr) { + OPENSSL_cleanse(plaintext, cryptogram_length + 1); + free(plaintext); + } + if (derived_key != nullptr) { + OPENSSL_cleanse(derived_key, key_size); + free(derived_key); + } return ret; } @@ -358,66 +468,83 @@ char* SJCL::encrypt(char* plaintext, const string& key) { salt = (unsigned char *) malloc(sizeof(unsigned char) * salt_len); derived_key = (unsigned char *) malloc(sizeof(unsigned char) * ks); - RAND_bytes(iv, iv_len); - RAND_bytes(salt, salt_len); - - // PBKDF2 Key derivation with SHA256 as SJCL does by default - PKCS5_PBKDF2_HMAC(key.c_str(), key.length(), salt, salt_len, iter, EVP_sha256(), ks, derived_key); - // Assuming ciphertext will not be bigger that the plaintext length * ciphertext_allocation_multiplicator ciphertext = (unsigned char *) malloc(sizeof(unsigned char) * strlen(plaintext) * ciphertext_allocation_multiplicator); - unsigned char *tmp_plaintext = reinterpret_cast(plaintext); - int ciphertext_len = encryptccm(tmp_plaintext, strlen(plaintext), additional, strlen ((char *)additional), derived_key, iv, iv_len, ciphertext, tag, ts); + int ciphertext_len = -1; + if (iv != nullptr && salt != nullptr && derived_key != nullptr && ciphertext != nullptr) { + // Without random bytes, the IV and salt would be leftover memory and could repeat between encryptions + if (1 == RAND_bytes(iv, iv_len) && 1 == RAND_bytes(salt, salt_len)) { + // PBKDF2 Key derivation with SHA256 as SJCL does by default + if (1 == PKCS5_PBKDF2_HMAC(key.c_str(), key.length(), salt, salt_len, iter, EVP_sha256(), ks, derived_key)) { + unsigned char *tmp_plaintext = reinterpret_cast(plaintext); + ciphertext_len = encryptccm(tmp_plaintext, strlen(plaintext), additional, strlen ((char *)additional), derived_key, iv, iv_len, ciphertext, tag, ts); + } else { + handleErrors("Error deriving the key"); + } + } else { + handleErrors("Error generating the IV and salt"); + } + } else { + handleErrors("Out of memory"); + } + if (0 < ciphertext_len) { + // without memory for the ciphertext with tag, the missing ciphertext_base64 fails the encryption below + Datagram* ciphertext_base64 = nullptr; uint8_t *ciphertext_with_tag = static_cast(malloc(sizeof(char *) * (ciphertext_len + ts))); - memcpy(ciphertext_with_tag, ciphertext, ciphertext_len); - memcpy(ciphertext_with_tag + ciphertext_len, tag, ts); + if (ciphertext_with_tag != nullptr) { + memcpy(ciphertext_with_tag, ciphertext, ciphertext_len); + memcpy(ciphertext_with_tag + ciphertext_len, tag, ts); - Datagram* ciphertext_base64 = BASE64::encode( - reinterpret_cast(ciphertext_with_tag), ciphertext_len + ts); - char *tmpct64 = reinterpret_cast(ciphertext_base64->data); - free(ciphertext_with_tag); + ciphertext_base64 = BASE64::encode( + reinterpret_cast(ciphertext_with_tag), ciphertext_len + ts); + free(ciphertext_with_tag); + } Datagram* salt_base64 = BASE64::encode(salt, salt_len); - char *tmpsalt64 = reinterpret_cast(salt_base64->data); - Datagram* iv_base64 = BASE64::encode(iv, iv_len); - char *tmpiv64 = reinterpret_cast(iv_base64->data); - - - nlohmann::json food; - - food["ct"] = tmpct64; - food["salt"] = tmpsalt64; - food["iv"] = tmpiv64; - food["v"] = 1; - food["iter"] = iter; - food["ks"] = key_size; - food["ts"] = tag_size; - food["mode"] = "ccm"; - food["adata"] = ""; - food["cipher"] = "aes"; - - string retrn = food.dump(); - ret = (char *) std::calloc(retrn.length() +1, sizeof(char)); - strcpy(ret, retrn.c_str()); + // ensure all three encodings worked before continuing + if (ciphertext_base64 != nullptr && salt_base64 != nullptr && iv_base64 != nullptr) { + nlohmann::json food; + + food["ct"] = reinterpret_cast(ciphertext_base64->data); + food["salt"] = reinterpret_cast(salt_base64->data); + food["iv"] = reinterpret_cast(iv_base64->data); + + food["v"] = 1; + food["iter"] = iter; + food["ks"] = key_size; + food["ts"] = tag_size; + food["mode"] = "ccm"; + food["adata"] = ""; + food["cipher"] = "aes"; + + string retrn = food.dump(); + ret = (char *) std::calloc(retrn.length() +1, sizeof(char)); + if (ret != nullptr) { + strcpy(ret, retrn.c_str()); + } else { + handleErrors("Out of memory"); + } + } else { + handleErrors("Error encoding the cryptogram"); + } - free(ciphertext_base64->data); - delete ciphertext_base64; - free(salt_base64->data); - delete salt_base64; - free(iv_base64->data); - delete iv_base64; + releaseDatagram(ciphertext_base64); + releaseDatagram(salt_base64); + releaseDatagram(iv_base64); } // Free up resources, wipe the secrets before free(ciphertext); free(iv); free(salt); - OPENSSL_cleanse(derived_key, ks); - free(derived_key); + if (derived_key != nullptr) { + OPENSSL_cleanse(derived_key, ks); + free(derived_key); + } return ret; } diff --git a/app/src/main/cpp/base64.cpp b/app/src/main/cpp/base64.cpp index 7c02dbe2..ac1af2e6 100644 --- a/app/src/main/cpp/base64.cpp +++ b/app/src/main/cpp/base64.cpp @@ -63,13 +63,15 @@ Datagram* BASE64::decode(const unsigned char *str, int length) { ret->data = (unsigned char *)malloc(length+1); if (ret->data == NULL) { fprintf(stderr, "out of memory!\n"); - exit(1); + delete ret; + return NULL; } while ((ch = *current++) != '\0' && length-- > 0) { if (ch == base64_pad) { if (*current != '=' && (i % 4) == 1) { free(ret->data); + delete ret; return NULL; } continue; @@ -80,6 +82,7 @@ Datagram* BASE64::decode(const unsigned char *str, int length) { continue; } else if (ch == -2) { free(ret->data); + delete ret; return NULL; } @@ -108,6 +111,7 @@ Datagram* BASE64::decode(const unsigned char *str, int length) { switch(i % 4) { case 1: free(ret->data); + delete ret; return NULL; case 2: k++; @@ -127,15 +131,15 @@ Datagram* BASE64::encode(const unsigned char *str, int length) { unsigned char *p; if ((length + 2) < 0 || ((length + 2) / 3) >= (1 << (sizeof(int) * 8 - 2))) { - ret->data = NULL; - ret->length = 0; - return ret; + delete ret; + return NULL; } ret->data = (unsigned char *)malloc((((length + 2) / 3) * 4)*(sizeof(char))+(1)); if (ret->data == NULL) { fprintf(stderr, "out of memory!\n"); - exit(1); + delete ret; + return NULL; } p = ret->data; diff --git a/app/src/main/cpp/passman-lib.cpp b/app/src/main/cpp/passman-lib.cpp index dcb443c5..952559d4 100644 --- a/app/src/main/cpp/passman-lib.cpp +++ b/app/src/main/cpp/passman-lib.cpp @@ -66,15 +66,20 @@ void wipe(std::string &secret) OPENSSL_cleanse(&secret[0], secret.size()); } -extern "C" { - -jstring Java_es_wolfi_app_passman_SJCLCrypto_decryptStringCpp(JNIEnv *env, jclass jthis, jstring cryptogram, jstring key) { +jstring decryptString(JNIEnv *env, jstring cryptogram, jstring key) { std::string dt = toStdString(env, cryptogram); std::string password = toStdString(env, key); WLF::Crypto::Datagram *t = WLF::Crypto::BASE64::decode((const unsigned char *) dt.c_str(), dt.length()); + if (t == NULL) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "error decoding the cryptogram"); + wipe(password); + throwJavaException(env, "Error decrypting"); + return NULL; + } + std::string json = (char *) t->data; free(t->data); delete t; @@ -98,10 +103,10 @@ jstring Java_es_wolfi_app_passman_SJCLCrypto_decryptStringCpp(JNIEnv *env, jclas } throwJavaException(env, "Error decrypting"); - return (jstring) ""; + return NULL; } -jstring Java_es_wolfi_app_passman_SJCLCrypto_encryptStringCpp(JNIEnv *env, jclass jthis, jstring plaintext, jstring key) { +jstring encryptString(JNIEnv *env, jstring plaintext, jstring key) { std::string dt = toStdString(env, plaintext); std::string password = toStdString(env, key); @@ -112,9 +117,6 @@ jstring Java_es_wolfi_app_passman_SJCLCrypto_encryptStringCpp(JNIEnv *env, jclas wipe(password); //__android_log_write(ANDROID_LOG_ERROR, LOG_TAG, result); - char *arr_result = &result[0]; - std::string lengthAsString = to_string(strlen(arr_result)); - if (result == NULL) { __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "error encrypting"); } @@ -122,18 +124,48 @@ jstring Java_es_wolfi_app_passman_SJCLCrypto_encryptStringCpp(JNIEnv *env, jclas //__android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "WHOOP!"); //__android_log_write(ANDROID_LOG_ERROR, LOG_TAG, result); WLF::Crypto::Datagram *t = WLF::Crypto::BASE64::encode((const unsigned char *) result, - strlen(arr_result)); + strlen(result)); + free(result); - jstring str = env->NewStringUTF((char *) t->data); + if (t != NULL) { + jstring str = env->NewStringUTF((char *) t->data); - free(t->data); - delete t; - free(result); - return str; + free(t->data); + delete t; + return str; + } + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "error encoding the cryptogram"); } throwJavaException(env, "Error encrypting"); return NULL; } -} \ No newline at end of file +/* + * A C++ exception leaving a JNI function aborts the app, e.g. the std::bad_alloc of a failed allocation + * in std::string or the JSON libraries, so the JNI functions turn it into the usual Java exception. + * (hopefully fixing a lot of random crashes, reported in the google play console) + */ +extern "C" { + +jstring Java_es_wolfi_app_passman_SJCLCrypto_decryptStringCpp(JNIEnv *env, jclass jthis, jstring cryptogram, jstring key) { + try { + return decryptString(env, cryptogram, key); + } catch (...) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "unexpected error decrypting"); + throwJavaException(env, "Error decrypting"); + return NULL; + } +} + +jstring Java_es_wolfi_app_passman_SJCLCrypto_encryptStringCpp(JNIEnv *env, jclass jthis, jstring plaintext, jstring key) { + try { + return encryptString(env, plaintext, key); + } catch (...) { + __android_log_write(ANDROID_LOG_ERROR, LOG_TAG, "unexpected error encrypting"); + throwJavaException(env, "Error encrypting"); + return NULL; + } +} + +}