IndexGetKeyRequestOp::GetResponse and ObjectStoreGetKeyRequestOp::GetResponse always return a 0 response size in the getAll case
Categories
(Core :: Storage: IndexedDB, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: sg, Assigned: sg)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
In https://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#24923, mResponse is swapped with a newly initialized, thus empty array, and then the size of its elements in calculated, which is always 0. This must be done either before swapping, or on the target array.
The same problem exists in IndexGetKeyRequestOp::GetResponse.
The effect of this is that the check for the maximum message size with always succeed.
Assignee | ||
Comment 1•5 years ago
|
||
I am not sure if this is a security issue, since I don't know why the limit on the message size exists. I marked it as a security issue for now to be sure.
Assignee | ||
Comment 2•5 years ago
|
||
Before this, the response size was always calculated as 0, since it used the swapped-out array.
In addition, make use of std::accumulate to increase idiomacy.
Assignee | ||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Hi Freddy, we discussed this bug in our meeting that this is a eviltraps bug, instead of a security one. Do you think you can help us to unhide this? thanks!
Comment 4•5 years ago
|
||
I'm confused. How is this an evil trap? It does not seem like this is annoying to users or even useful to websites that want to scare a user?
However, if you're sure this isn't a security bug, I'm happy to unhide.
Assignee | ||
Comment 5•5 years ago
|
||
As said before, I was not sure if this kind of bug is a security bug. After discussion in the team, it turned out that the effect would be as follows:
- The response size for queries querying all keys in an object store or index is always reported as 0.
- Normally, responses exceeding the maximum IPC message limit are caught within ActorsParent.cpp and turned into a IDB error, resulting is JS exception for the caller.
- If a query were issued that resulted in a response size exceeding the maximum IPC message limit, this would instead tried to be passed to the IPC layer, which would detect this and crash the parent process.
Comment 6•5 years ago
|
||
I agree with Freddy that this isn't an evil trap. The point of evil trap bugs is DOS-style attacks that make it impossible for users to interact with the browser or close/leave a page. This is a very different type of bug.
Assignee | ||
Comment 7•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #6)
I agree with Freddy that this isn't an evil trap. The point of evil trap bugs is DOS-style attacks that make it impossible for users to interact with the browser or close/leave a page. This is a very different type of bug.
Ok, sorry, then I was misinterpreting something here.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d23a0a0ffa93
Correctly calculate response size in IndexGetKeyRequestOp::GetResponse and ObjectStoreGetKeyRequestOp::GetResponse r=ttung,asuth
Comment 9•5 years ago
|
||
bugherder |
Description
•