Closed
Bug 741872
Opened 12 years ago
Closed 8 years ago
[Security Review][Action Item]WebSMS - sqlite queries
Categories
(mozilla.org :: Security Assurance, task)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: curtisk, Assigned: pauljt)
References
()
Details
(Whiteboard: [start 04/18/2012][target 04/25/2012])
audit sqlite interface to ensure parameterized queries and other SQLi defenses (I guess this is just the Fennec version?).
Assignee | ||
Updated•12 years ago
|
Whiteboard: [start 04/18/2012][target 04/25/2012]
Assignee | ||
Comment 1•12 years ago
|
||
Not sure where the sqlite information came from , but AFAICT sms on android uses a ContentProvider to store and retrieve sms messages. Data is added using the .insert() method, which isn't string based, so don't think there is an injection risk there. Retrieval use query, which is good, but it is still possible to have injections in selection lists. This code worries me: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoSmsManager.java#775 Especially this line, since mNumbers is a String Array, maybe an injection risk (i don't really know enough about how content providers work)? http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoSmsManager.java#787 Not sure where mNumbers comes from - my guess would be be a user choosing a number from the interface, rather than simply typing one in. Even if it was user input, I expect it would come from the user, not from an sms message, so remote exploitation risk I think is low. Mounir, some questions: 1- Can you confirm that SQLlite is not used on android (just making sure I didn't miss it) 2 - Do you see any security issues with the code at http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoSmsManager.java#775
Comment 2•12 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #1) > Mounir, some questions: > > 1- Can you confirm that SQLlite is not used on android (just making sure I > didn't miss it) AFAIK, ContentProvider is a a wrapper around a SQLite database, see [1]. > 2 - Do you see any security issues with the code at > http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ > GeckoSmsManager.java#775 selectionArgs is the recommended way to prevent SQL injection [1] so, yes, that's why I put that comment. Note that mNumbers comes from the content where a web developer can create a SmsFilter object with a list of numbers to filter against. [1] http://developer.android.com/guide/topics/providers/content-provider-basics.html
Assignee | ||
Comment 3•12 years ago
|
||
Ok thanks Mounir, I wasn't sure about what was behind the Content Provider code. I'll guess I will just leave this open then until that gets patched. I was thinking there might be mitigating controls, like maybe only the sms database might be affected, and the component would have full access to the sms database anyways. But its probably easier just to fix it and remove the risk. (And I suppose there may be a use case where an application might have access to read sms but not modify the database.
Comment 4•12 years ago
|
||
Actually, consumer of the API are allowed to read SMS or delete them but they can't modify them so we clearly don't want an app to have full access to the database. IIRC, there is no bug open on that issue.
Assignee | ||
Updated•12 years ago
|
Blocks: B2G-secreview
Assignee | ||
Updated•12 years ago
|
No longer blocks: B2G-secreview
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•