Last Comment Bug 721129 - Incorrect BigInteger comparison in JPakeCrypto
: Incorrect BigInteger comparison in JPakeCrypto
Status: RESOLVED FIXED
[qa-]
:
Product: Android Background Services
Classification: Client Software
Component: Android Sync (show other bugs)
: unspecified
: ARM Android
: -- normal
: ---
Assigned To: Richard Newman [:rnewman]
:
:
Mentors:
Depends on:
Blocks: 720933 723230
  Show dependency treegraph
 
Reported: 2012-01-25 11:07 PST by David Chan [:dchan]
Modified: 2013-04-04 13:48 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Proposed patch. v1 (2.44 KB, patch)
2012-01-25 12:14 PST, Richard Newman [:rnewman]
dchanm+bugzilla: review+
Details | Diff | Splinter Review

Description David Chan [:dchan] 2012-01-25 11:07:13 PST
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] 2012-01-25 12:14:22 PST
Created attachment 591580 [details] [diff] [review]
Proposed patch. v1

Fix and test. Should be a rubberstamp…! :D
Comment 2 David Chan [:dchan] 2012-01-25 12:55:55 PST
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] 2012-01-25 13:00:46 PST
Fixed in develop:

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

Thanks for the quick review!
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2012-01-27 23:51:40 PST
Richard, please request aurora approval
Comment 5 Richard Newman [:rnewman] 2012-01-27 23:57:11 PST
(In reply to Brad Lassey [:blassey] from comment #4)
> Richard, please request aurora approval

Already merged as part of Bug 720933.

Note You need to log in before you can comment on or make changes to this bug.