Incorrect BigInteger comparison in JPakeCrypto

RESOLVED FIXED

Status

Android Background Services
Android Sync
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: dchan, Assigned: rnewman)

Tracking

unspecified
ARM
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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
Created attachment 591580 [details] [diff] [review]
Proposed patch. v1

Fix and test. Should be a rubberstamp…! :D
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #591580 - Flags: review?(dchan+bugzilla)
Blocks: 720933
(Reporter)

Comment 2

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

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

Hurrah! Patch with a test.
Attachment #591580 - Flags: review?(dchan+bugzilla) → review+
Fixed in develop:

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

Thanks for the quick review!
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Richard, please request aurora approval
(In reply to Brad Lassey [:blassey] from comment #4)
> Richard, please request aurora approval

Already merged as part of Bug 720933.
status-firefox11: --- → fixed

Updated

5 years ago
Whiteboard: [qa-]
(Reporter)

Updated

5 years ago
Blocks: 723230
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services
You need to log in before you can comment on or make changes to this bug.