722496 - JPakeCrypto.java potential null pointer dereferences

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P1)

Description

[David Chan [:dchan]]

If secret.getBytes() fails, then y2 will be undefined and y1.modPow(y2, P); will throw an exception
[1]

If MessageDisgest.getInstance() or id.getBytes() fail then an exception will be thrown at sha.digsst() or BigIntegerHelper.ByteArrayToBigIntegerWithoutSign
[2][3]

The code should abort / return error in these cases.

[1] - https://github.com/mozilla-services/android-sync/blob/master/src/main/java/org/mozilla/gecko/sync/jpake/JPakeCrypto.java#L153
[2] - https://github.com/mozilla-services/android-sync/blob/master/src/main/java/org/mozilla/gecko/sync/jpake/JPakeCrypto.java#L297
[3] - https://github.com/mozilla-services/android-sync/blob/master/src/main/java/org/mozilla/gecko/sync/jpake/BigIntegerHelper.java#L45

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The parameter should not be a string, but a byte array. Whatever conversion from string to byte array that might be necessary should happen in the non-crypto code.

Comment 2

[Richard Newman [:rnewman]]

Fixed in develop:

https://github.com/mozilla-services/android-sync/commit/7721c1c50f28113ddd0db235894175e317c41a41

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1bfbf78359af