791133 - Security Coverage: Inspect several indexedDB files

Status: RESOLVED INCOMPLETE

(Core :: Storage: IndexedDB, defect)

Description

[Christian Holler (:decoder)]

According to our analysis, several files in indexedDB have (small) untested portions of code/functions while the subsystem has been patched one or more times in the last 6 months due to security problems. The coverage data for these files (from a try run with our main test suites) is here:

http://people.mozilla.org/~choller/coverage/mc-tests-all-20120903/dom/indexedDB/index.html

(Assertions, warnings and errors like out-of-memory conditions or those that cannot be triggered through content should be ignored).

In particular, I'd like to highlight the following files because they seem to interact directly with content:

dom/indexedDB/IDBKeyRange.cpp: code around line 320, methods starting at line 349
dom/indexedDB/IDBObjectStore.cpp: code around lines 464, 608, 1173, 3232 and 3745
dom/indexedDB/TransactionThreadPool.cpp: code around lines 170 and 539

Furthermore, the files dom/indexedDB/IDBFileHandle.cpp and dom/indexedDB/FileManager.cpp have large coverage holes, but I don't know how relevant that code is for content.

Please add appropriate tests and/or check the untested portions of code if it is possible and reasonable.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Thanks for this data!

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

IDBKeyRange.cpp
Yeah, we need tests for key ranges.  We don't have much (see e.g. Bug 738985).

IDBObjectStore.cpp
line 464: The only way this codepath is taken is if DispatchToTransactionThreadpool fails, so this is probably not possible to hit reliably in practice.
line 608: IPC stuff for bent to test
line 1173: same
line 3232: need to add something to a cursor test (we're covering the identical branch for NEXT, just not PREV)
line 3745: Apparently we don't have any tests for getAll?

TransactionThreadPool.cpp
line 170: This requires a shutdown to test.  Maybe we could write an xpcshell test?
line 539: we should be able to test this.

IDBFileHandle.cpp/FileManager.cpp
These definitely need better tests.  Janv?

Comment 3

[Jan Varga [:janv]]

(In reply to Kyle Huey [:khuey] ([e-mail]) [Away until September 18th (MozCamp EU, then vacation)] from comment #2)
> 
> IDBFileHandle.cpp/FileManager.cpp
> These definitely need better tests.  Janv?

yeah, I should do that

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Per discussion with decoder, these bugs are not likely to be relevant at this point (and the reports themselves are long gone). Any further work is better suited for new bugs at this point.