mogol/flutter_secure_storage

App uses CBC with PKCS5/PKCS7 padding

Den-creator opened this issue · 7 comments

Mobile Security Framework (MobSF) reports high risk error that comes from the android part of package.

Error details:
The App uses the encryption mode CBC with PKCS5/PKCS7 padding. This configuration is vulnerable to padding oracle attacks.

CWE: CWE-649: Reliance on Obfuscation or Encryption of Security-Relevant Inputs without Integrity Checking
OWASP Top 10: M5: Insufficient Cryptography
OWASP MASVS: MSTG-CRYPTO-3

Source of error: c5/h.java

    protected Cipher d() {
        return Cipher.getInstance("AES/CBC/PKCS7Padding");
    }

I see a lot of similar reports:
#584
#562
#526
#475
They are either ignored or closed, but issue is still persists.

The provided solution:

const AndroidOptions(
        encryptedSharedPreferences: true,
      storageCipherAlgorithm: StorageCipherAlgorithm.AES_GCM_NoPadding
      );

is good to have, but it doesn't remove the warning reported by MobSF.
Why Cipher.getInstance("AES/CBC/PKCS7Padding") is still in the package ? Why will we not just remove it ?

I would really appreciate if you would have a look at this issue and help to resolve it. Many thanks !

@juliansteenbakker additionally, could you please tell whether provided by you code

const AndroidOptions(
        encryptedSharedPreferences: true,
      storageCipherAlgorithm: StorageCipherAlgorithm.AES_GCM_NoPadding
      );

ref

makes sense, as library have such comments:

/// If EncryptedSharedPrefences is set to false, you can select algorithm
/// that will be used to encrypt properties.
/// By default AES/CBC/PKCS7Padding if used.
/// Newer AES/GCM/NoPadding is available from Android 6.
/// Plugin will fall back to default algorithm in previous system versions.
final StorageCipherAlgorithm _storageCipherAlgorithm;

static AndroidOptions _getAndroidOptions() => const AndroidOptions(
   keyCipherAlgorithm: KeyCipherAlgorithm.RSA_ECB_OAEPwithSHA_256andMGF1Padding,
   storageCipherAlgorithm: StorageCipherAlgorithm.AES_GCM_NoPadding,
 );
 static final storage = FlutterSecureStorage(aOptions: _getAndroidOptions());
 static writeSecureData(String key, String value) async {
   await storage.write(key: key, value: value);
 }
 static Future<String> readSecureData(String key) async {
   return await storage.read(key: key) ?? 'No data found';
 }

I am using this code but still get the same issue.

In my opinion, they are trying to make the package available for Android versions lower than API 23 (Android 6), but this is a security data storage package and versions prior to this API are vulnerable.

That said, there's no way to provide security for users using insecure devices, I think this should be removed from the source code and it should be mandatory to use the "minSdkVersion" flag as 23 or higher.

I look forward to hearing from you, so that I can decide whether to continue using this package or switch to a more secure and reliable one.

In my opinion, they are trying to make the package available for Android versions lower than API 23 (Android 6), but this is a security data storage package and versions prior to this API are vulnerable.

That said, there's no way to provide security for users using insecure devices, I think this should be removed from the source code and it should be mandatory to use the "minSdkVersion" flag as 23 or higher.

I look forward to hearing from you, so that I can decide whether to continue using this package or switch to a more secure and reliable one.

so what is the most secure and reliable way for me to use because we are developing mobile app and we are stuggling when we scan the app in mobsf and receive a vuln indicating PKCS7 PADDING is vuln to oracle padding attack

In my opinion, they are trying to make the package available for Android versions lower than API 23 (Android 6), but this is a security data storage package and versions prior to this API are vulnerable.
That said, there's no way to provide security for users using insecure devices, I think this should be removed from the source code and it should be mandatory to use the "minSdkVersion" flag as 23 or higher.
I look forward to hearing from you, so that I can decide whether to continue using this package or switch to a more secure and reliable one.

so what is the most secure and reliable way for me to use because we are developing mobile app and we are stuggling when we scan the app in mobsf and receive a vuln indicating PKCS7 PADDING is vuln to oracle padding attack

See if you are setting a different encryption than the default, if so just ignore this alert, another option is to create a fork and make the changes for your application

In my opinion, they are trying to make the package available for Android versions lower than API 23 (Android 6), but this is a security data storage package and versions prior to this API are vulnerable.

That said, there's no way to provide security for users using insecure devices, I think this should be removed from the source code and it should be mandatory to use the "minSdkVersion" flag as 23 or higher.

I look forward to hearing from you, so that I can decide whether to continue using this package or switch to a more secure and reliable one.

Which would be the Secure Huawei Libraries to use in this Case

I am working on a new version of this package that fixes these problems. It will be tracked in #769