1813583 - Out-Of-Bounds Access in ConvertSidToBytes

Status: RESOLVED INVALID

(Core :: Audio/Video: GMP, defect)

Description

[Kirtikumar Anandrao Ramchandani]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/109.0.0.0 Safari/537.36

Steps to reproduce:

This is found during the manual source code analysis. Please find the details below:
https://searchfox.org/mozilla-central/source/dom/media/gmp/rlz/win/lib/machine_id_win.cc#107

On line 107, it will cause OOB Access. It is quite weird how it will cause OOB when it is trying to retrieve information about a Windows computer for use in an identifier or ID

Actual results:

Out-Of-Bound Access

Expected results:

https://source.chromium.org/chromium/chromium/src/+/main:rlz/win/lib/machine_id_win.cc;l=18;bpv=0

Comment 1

[Kirtikumar Anandrao Ramchandani]

I think there are multiple out-of-bounds lying in that directory. One more can be found below:
https://searchfox.org/mozilla-central/source/dom/media/gmp/rlz/mac/lib/machine_id_mac.cc#314

In this, I think the idsize() in the data->assign(&id[0], &id[id.size()]); will go out of bounds. Because ifid.size()returns a value that is larger than the actual size of theidvector, accessing the element&id[id.size()], it would result in OOB. After which thedata->assign(&id[0], &id[id.size()]);` will attempt to copy elements from an invalid memory location.

Please bisect the bug if you can verify this as well.

Comment 2

[Andrew McCreight [:mccr8]]

Thanks for the report. Hopefully somebody who is familiar with this code will be able to look at this bug soon.

Comment 3

[Kirtikumar Anandrao Ramchandani]

Thanks!

Comment 4

[Andrew McCreight [:mccr8]]

I looked at this. sid_string has type std::wstring. size is the number of characters in the string. That specific constructor for std::vector works as follows: "Constructs the container with the contents of the range [first, last).". In other words, it is not copying the size()th element of the array, but only up to one before it. This code is written correctly as far as I can tell.