1741210 - Potential out-of-bounds write due to integer overflow [@ SnappyUncompress]

Status: RESOLVED FIXED

(Core :: Storage: localStorage & sessionStorage, defect, P2)

Description

[Christian Holler (:decoder)]

The Snappy code here

https://searchfox.org/mozilla-central/rev/3c8a7970944daaf917b547dffc0790bcd37cadc1/dom/localstorage/SnappyUtils.cpp#62

resizes nsACString dest with the uncompressed length of the data. Since the uncompressed length is larger than the compressed length, this can overflow when calling setLength which takes 32-bit for the length.

The result will be an out-of-bounds write while decompressing.

I talked to :janv about this and since local storage is limited to 5MB per origin, this shouldn't ever be exploitable from content. However, I don't know where this limit is enforced and maybe it is possible to circumvent that via IPC and feed larger data into the SnappyUtils.

Leaving unclassified until we have investigated this further.

Found with -Wshorten-64-to-32 with some custom filters on top of it + manual code inspection.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

LSNG was enabled by default for 92+, so I guess the status for ESR91 depends on whether this affects the old localstorage too or not?

Comment 2

[Jari Jalkanen]

Attachment: Bug 1741210 - Make type conversions more explicit. r=#dom-storage-reviewers

Comment 3

[Daniel Veditz [:dveditz]]

Where does the compressed data come from in the first place? I don't think a child could send a string through IPC that's long enough to trigger this, unless the compression is done on the child side. Or is the thing being uncompressed some big storage blob that's accumulated over many separate web content writes to storage?

If this data could come from the child this could lead to a sandbox escape that would be sec-high. But otherwise if we can't get strings big enough this might not be exploitable at all.

Comment 4

[Jari Jalkanen]

Local storage can use 5M of quota per origin, and the IPC messages are limited to 512M. The data field sizes in the underlying database are limited to the maximum signed 32-bit integer.

The compression header stores the uncompressed size as a 32-bit unsigned integer which is cast to size_t only on return, see https://searchfox.org/mozilla-central/source/other-licenses/snappy/src/snappy.cc#431 , https://searchfox.org/mozilla-central/source/other-licenses/snappy/src/snappy-stubs-internal.h#535 and another implementation https://searchfox.org/mozilla-central/source/other-licenses/snappy/src/snappy.cc#779

For me, it's not easy to see how to exploit this.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Make type conversions more explicit. r=dom-storage-reviewers,janv
https://hg.mozilla.org/integration/autoland/rev/a921076fbb77b77e0f67abf882dbfb72b6085a55
https://hg.mozilla.org/mozilla-central/rev/a921076fbb77