1544153 - OOBs related to sqlite3_malloc

Status: RESOLVED FIXED

(Toolkit :: Storage, defect, P3)

Description

[mlfbrown]

I reported this to Chrome today but I'm not sure what their process/SQLite's process is, so I'm copying the report here as well just in case:

VULNERABILITY DETAILS
Possible OOB with sqlite3_malloc

REPRODUCTION CASE
There's a pattern of using sqlite3_malloc in combination with traditional memory operations (e.g., memcpy). There may be invariants that make this ok, or a principle here that I am not aware of. Thanks for your time.

sqlite3_malloc takes an int size argument, while memcpy takes a size_t size argument. On x86-64 this means that sqlite3_malloc's size argument is width 32, while memcpy's is width 64. This can lead to potentially concerning wrapping behavior for extreme allocation sizes (depending on the compiler, optimizations, etc).

For example:

Function fts3UpdateDocTotals
(https://searchfox.org/mozilla-central/source/db/sqlite3/src/sqlite3.c#172846)

(1) a = sqlite3_malloc( (sizeof(u32)+10)*nStat );
(https://searchfox.org/mozilla-central/source/db/sqlite3/src/sqlite3.c#172863)

...
(2) memset(a, 0, sizeof(u32)*(nStat) );
(https://searchfox.org/mozilla-central/source/db/sqlite3/src/sqlite3.c#172881)

Depending on optimization level etc, this may turn into:

(1)
size = mul i32 nstat 14
sqlite3_malloc(size)

(2)
tmp = sign extend nstat to i64
size = shl tmp 2
memset(size)

If nstat is a very large i32, the multiplication in step (1) may wrap. Nothing in (2) will wrap because of the sign extend, leading to an OOB.

Comment 1

[:Gijs (he/him)]

Marco, I expect this needs reporting to sqlite?

Comment 2

[Marco Bonardo [:mak]]

yes, this should be reported upstream, it's not our code. I can Cc Richard here and notify them through the support e-mail.

Comment 3

[D. Richard Hipp]

There is a potential problem if an attacker is able to inject a CREATE VIRTUAL TABLE statement this is on the order of 250,000,000 bytes or longer in length. I do not think FF is vulnerable as I don't think there is any way for an attacker to inject any amount of SQL in FF since FF does not support WebSQL. A fix for this problem will appear in the next release of SQLite that is due out within a few days.

Comment 4

[Marco Bonardo [:mak]]

Thanks! I agree, we don't allow that kind of injection, we don't support WebSQL and we try to force binding parameters as far as possible, rather than building queries on the fly. In general we control all of the consumers, especially with WebExtensions being the status quo (in the past we could have been vulnerable through a legacy add-on).

Even if we're unaffected, I'm confirming this for tracking purposes and will be fixed upstream when we update to Sqlite 3.28.0. It may also be of interest for our security team.

Comment 5

[Marco Bonardo [:mak]]

Sqlite has been updated to 3.28.0

Comment 6

[Daniel Veditz [:dveditz]]

Unfortunately this does not qualify for the bug bounty program since the affected functionality is not exposed in Firefox.