scottyab / secure-preferences

Android Shared preference wrapper than encrypts the values of Shared Preferences. It's not bullet proof security but rather a quick win for incrementally making your android app more secure.
1.53k stars 235 forks source link

Wrong parameters order in one constructor #73

Open alaegin opened 6 years ago

alaegin commented 6 years ago

Hello!

I noted wrong parameters order in one of the constructors.

We have main one (line 130) - private SecurePreferences(Context context, final AesCbcWithIntegrity.SecretKeys secretKey, final String password, final String salt, final String sharedPrefFilename, int iterationCount) Params:

  1. context
  2. secretKey
  3. password
  4. salt
  5. sharedPrefFilename
  6. iterationCount

    Also, we have another one (line 126) - public SecurePreferences(Context context, final String password, final String salt, final String sharedPrefFilename, int iterationCount) { this(context, null, password, sharedPrefFilename, salt, iterationCount); }

If we see to "this()" call to main one we can note that we have this order:

  1. context
  2. secretKey
  3. password
  4. sharedPrefFilename
  5. salt
  6. iterationCount

Good. But, if we consider in detail, in main constructor we have

  1. salt (instead of sharedPrefFilename in second one)
  2. sharedPrefFilename (instead of salt in second one)

Screenshot

Maybe i'm wrong, but my xml files have are named with my password string and only after fixing order I got right file names.

Hope it helps. Regards, Alexey.

scottyab commented 6 years ago

Hey @DominuS-RU nice spot thanks. This will be fixed in 0.1.6

alaegin commented 6 years ago

One more mistake. Screenshot

We pass null salt as password and password as salt.

scottyab commented 6 years ago

to confirm this is the switch you meant @DominuS-RU?.

secure-preferences__git_
alaegin commented 6 years ago

@scottyab Yes, this is the swith for the first one. (The second screenshot is about another mistake)

Plinzen commented 6 years ago

Hi, is there a release date for this fix? In 0.1.6 it's still present:

     /**
     * @param context        should be ApplicationContext not Activity
     * @param iterationCount The iteration count for the keys generation
     */
    public SecurePreferences(Context context, final String password, final String salt, final String sharedPrefFilename, int iterationCount) {
        this(context, null, password, sharedPrefFilename, salt, iterationCount);
    }

    private SecurePreferences(Context context, final AesCbcWithIntegrity.SecretKeys secretKey, final String password, final String salt, final String sharedPrefFilename, int iterationCount) {
        if (sharedPreferences == null) {
            sharedPreferences = getSharedPreferenceFile(context, sharedPrefFilename);
        }
        ...
LosDanieloss commented 6 years ago

@scottyab Any progress on that?