1801542 - (CVE-2023-25731) Prototype pollution via GET request in network tool inspection

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

Attachment: img.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

Open developer tools, Network tab.
Open any website with query parameter __proto__=
Click on the request with our GET parameter, crash.

https://duckduckgo.com/?__proto__=x

I've confrimed this issue on v.105 and v.109-nightly

Actual results:

The Network panel has crashed.
TypeError: map[obj.name].push is not a function

Stacktrace (shortened):
parseUrl/urlObject[method].query<@resource://devtools/client/netmonitor/src/components/previews/UrlPreview.js:215:25
parseUrl@resource://devtools/client/netmonitor/src/components/previews/UrlPreview.js:208:40
render@resource://devtools/client/netmonitor/src/components/previews/UrlPreview.js:245:47

Expected results:

Request inspection as per usual

Comment 1

[school.exams]

Due to markdown underscores were made bold, in step to reproduce, query should be the following:
__proto__=

Comment 2

[Andrew McCreight [:mccr8]]

I've edited the first post to fix the markdown.

Comment 3

[Daniel Veditz [:dveditz]]

Can confirm.

In the Console tab if I try to expand the request it doesn't crash, but does give the following error

[DEVTOOLS ERROR] We’re sorry, we couldn’t render the message. This shouldn’t have happened - please file a bug at https://bugzilla.mozilla.org/enter_bug.cgi?product=DevTools&component=Console with the message metadata in the description.

The "copy message metadata to clipboard" button did not do anything.

The basic symptoms are annoying, but could this be used to actually redefine one of our prototypes in a malicious way? The Developer toolbox runs as privileged code. Setting a prototype to a string shouldn't be too bad... there isn't anything that will auto-eval this, right?

Comment 4

[school.exams]

I was unable to do anything actually impactful, but I marked it as a security issue because it seemed like unexplored attack surface and could come up somewhere else, like in third-party extensions (yeah, out of scope but still).

This bug was found while solving a CTF challenge, very curious when it was introduced as it seems so trivial.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can reproduce on my machine (MacOS, Nightly)
I see both mentioned problems. One in the Network panel and the other in the Console (also looks like the Console has better error handling of errors occurning in React components)

Feels like nicely actionable report (STRs as well as stack trace available)

Honza

Comment 6

[Andrew McCreight [:mccr8]]

As seen in the stack trace, the specific piece of code at issue is this one. This should use a Map instead of a bare object {} to reduce over.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:Honza, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Bomsy, could you please look at this? Looks actionable, thank you.

Comment 9

[Hubert Boma Manilla (:bomsy)]

Oh sure!

i'll take a look. Thanks Honza!

Comment 10

[Hubert Boma Manilla (:bomsy)]

Attachment: Bug 1801542 - Fix prototype pollution when rendering the UrlPreview r=nchevobbe

As the since the object that is used to consolidate the query for the url
has its prototype chain, adding a parameter with name __proto__ tries to lookup the
prototype chain and blows up.

Lets amke it a simple dictionary by removing the prototype chain.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bomsy, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 12

[Julian Descottes [:jdescottes]]

This will probably land when :bomsy will come back from PTO.

Comment 13

[Hubert Boma Manilla (:bomsy)]

This should land today.

Comment 14

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

Fix prototype pollution when rendering the UrlPreview r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/4343b20a43027647e39c1b3f0f01aabfe45b0ffe
https://hg.mozilla.org/mozilla-central/rev/4343b20a4302

Comment 15

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 16

[school.exams]

Sorry if it might be too late to ask this: can you add my colleague to the advisory as well?

His name is Alexander Volkov, while we were playing the CTF we both found this independently within minutes of each other. Also, I should have used my primary email address, can you change "school.exams" to "pyakovlev"?

So that the new "reported by" field in the advisory goes like this:

pyakovlev & Alexander Volkov

Thanks in advance!

Comment 17

[June Wilde (she/her) [:jewilde]]

I've updated the advisory in our back end so the reporter will show up as pyakovlev & Alexander Volkov, thanks!

Comment 18

[Daniel Veditz [:dveditz]]

Sorry for the burst of bugspam: filter on tinkling-glitter-filtrate
Adding reporter-external keyword to security bugs found by non-employees for accounting reasons