716143 - SQL injection flaws in storage layer

Status: RESOLVED FIXED

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

Description

[Richard Newman [:rnewman]]

Thanks to imelven for the review!

---
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java#90

89   protected void delete(String guid) {
90     context.getContentResolver().delete(getUri(), BrowserContract.SyncColumns.GUID + " = '" + guid + "'", null);
91   }


potential injection there and also same pattern at 
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java#104
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java#142
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java#160
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java#164
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java#159

Would prefer this was along the lines of 

context.getContentResolver().delete(getUri(), BrowserContract.SyncColumns.GUID + " = ?", new String[] { guid });

just to be totally safe. 

Not super concerned about it since these are GUIDs and should always 
be coming from sync and not content but i'm paid to be this paranoid... :D

Comment 1

[Marina Samuel [:emtwo]]

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

Comment 2

[Richard Newman [:rnewman]]

(In reply to Marina Samuel [:marina] from comment #1)
> Resolved:
> https://github.com/mozilla-services/android-sync/commit/
> d4010c878568e39290ccc827f96b8c754098569a

Wrong bug? Did you mean Bug 716073?

Comment 3

[Marina Samuel [:emtwo]]

Yes! Please ignore comment 1 :)

Comment 4

[Marina Samuel [:emtwo]]

This is still not complete but I'd like some feedback for thus far please :)

Fix:
https://github.com/mozilla-services/android-sync/commit/211af1

Tests:
https://github.com/mozilla-services/test-android-sync/commit/44523

Comment 5

[Marina Samuel [:emtwo]]

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

Tests:
https://github.com/mozilla-services/test-android-sync/commit/3afcea

Comment 6

[Richard Newman [:rnewman]]

Is this complete, Marina?

Comment 7

[Marina Samuel [:emtwo]]

Yes