721129 - Incorrect BigInteger comparison in JPakeCrypto

Status: RESOLVED FIXED

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

Description

[David Chan [:dchan]]

JPakeCrypto.java [1]
141     if (jp.gx4 == BigInteger.ONE) {
142       throw new Gx4IsOneException();
143     }

The above comparison will fail do to reference checking. jp.gx4 is assigned in JPakeClient.java [2] with new BigInteger(String) .

[1] - https://github.com/mozilla-services/android-sync/blob/develop/src/main/java/org/mozilla/gecko/sync/jpake/JPakeCrypto.java#L141
[2] - https://github.com/mozilla-services/android-sync/blob/develop/src/main/java/org/mozilla/gecko/sync/jpake/JPakeClient.java#L354

Comment 1

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Fix and test. Should be a rubberstamp…! :D

Comment 2

[David Chan [:dchan]]

Comment on attachment 591580 [details] [diff] [review]
Proposed patch. v1

Review of attachment 591580 [details] [diff] [review]:
-----------------------------------------------------------------

Hurrah! Patch with a test.

Comment 3

[Richard Newman [:rnewman]]

Fixed in develop:

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

Thanks for the quick review!

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Richard, please request aurora approval

Comment 5

[Richard Newman [:rnewman]]

(In reply to Brad Lassey [:blassey] from comment #4)
> Richard, please request aurora approval

Already merged as part of Bug 720933.