Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Potential SQL injection in /netwerk/cache/src/nsDiskCacheDeviceSQL.cpp

RESOLVED FIXED

Status

()

Core
Networking: Cache
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Thor Larholm, Assigned: mayhemer)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

Most of the SQL statements in nsDiskCacheDeviceSQL.cpp that accept some kind of input from the outside world are defined in the prepared structure, e.g.:

{ mStatement_DeleteEntry,     "DELETE FROM moz_cache WHERE ClientID = ? AND Key = ?;" }

These prepared statements are all executed with statement->ExecuteStep after having their paramters bound with BindDataParameter/BindCStringParameter/etc, which properly guards against SQL injections.

Aside from these, there are 5 calls to mDB->ExecuteSimpleSQL - of which one accepts input and does not have its input properly bound. See http://lxr.mozilla.org/seamonkey/source/netwerk/cache/src/nsDiskCacheDeviceSQL.cpp#1228

The problematic piece of code is 

    deleteCmd =
      PR_smprintf("DELETE FROM moz_cache WHERE ClientID=%q AND Flags=0;",
                  clientID);

PR_smprintf simply adds quotes around the input. If you can control clientID and have it contain quotes you can inject and execute arbitrary SQL.

I've written "Potential SQL injection" in the summary as I haven't looked deeply into all the places where nsDiskCacheDevice::EvictEntries is called. See http://lxr.mozilla.org/seamonkey/ident?i=EvictEntries

If you can successfully inject SQL you can alter the content of the disk and memory cache as well as read/write files to the local system through ATTACH/CREATE.


Reproducible: Always

Steps to Reproduce:
1. Provide EvictEntries with a clientID char that contains quotes
2. Wait for mDB->ExecuteSimpleSQL
3. SQL Injection
(Reporter)

Comment 1

11 years ago
Did anyone take a look at this? It's still labelled UNCONFIRMED

Comment 2

11 years ago
nsDiskCacheDeviceSQL.cpp is not used in any Mozilla product.  It just happens to live in the source tree as an experimental disk cache implementation.

Comment 3

11 years ago
since this code is not shipping in gecko, i see no reason to keep this bug confidential.
Group: security
(Reporter)

Comment 4

11 years ago
Indeed, the default configure contains NECKO_DISK_CACHE=1 instead of NECKO_DISK_CACHE_SQL=1 so nsCacheService.cpp never gets to use the SQLite implementation of the cache. Even Firefox 2 doesn't use it, but still has the source in it's tree - any reason to even keeping it there? :)

Comment 5

11 years ago
presumably someone was hoping to eventually switch to it. if they choose to do that, they'll be required to fix this before they turn it on. in the interim, anyone including you could post a patch for it. presumably darin would gladly review it.

Comment 6

8 years ago
> presumably someone was hoping to eventually switch to it. if they choose to do
> that, they'll be required to fix this before they turn it on.
The offline cache already uses nsDiskCacheDeviceSQL...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 7

8 years ago
The ClientID is built from the URL of the manifest, that must be a valid URL and is defined in <html manifest=> tag.  So, there is a potential to exploit this.

Going to fix this as it should be quit simple.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
(Assignee)

Comment 8

8 years ago
This has been fixed as part of bug 442806, http://hg.mozilla.org/mozilla-central/rev/3e41c4b388c3.

Should we back port only the fix for this issue to 1.9.0?

Comment 9

8 years ago
If it's easy, probably worth it.
Was this patch backported?

Comment 11

6 years ago
(In reply to Marco Castelluccio from comment #10)
The fix from bug 442806 wasn't backported to Gecko 1.9.0, which is equivalent to Firefox 3.0, because that's way past end-of-life. Gecko 1.9.1/Firefox 3.5 contains the fix, but is past end-of-life as well. Firefox 3.6.24 is still supported if you have to run such an old browser.

Can we mark this bug as FIXED (by bug 442806, according to comment 8)?
Depends on: 442806
(Assignee)

Comment 12

6 years ago
Fixed by bug 442806.  Fix contained since 1.9.1.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.