1888847 - DevTools Storage inspector cookie table rendering issue/misalignment with tall characters

Status: RESOLVED FIXED

(DevTools :: Storage Inspector, defect, P3)

Description

[matan.honig2]

Attachment: cookies_POC.html

I was able to reproduce the vulnerability also in Windows 10, Windows 11 and Fedora Linux. Firefox version: 123.0.1

The issue is: an attacker site can use set-cookie to "log-inject" the developer-console cookie table (Storage>Cookies) by using tall Unicode characters.
Assuming an attacker was able to control (XSS or subdomain takeover) the victim website, and victim website has a specific Cookie which is Http-only, so the user can look at it. The attacker might use this technic to bypass cookies table layer of protection, and "inject" his value into the table in front of the victim value.

Run the POC HTML file with an HTTP server (e.g. python -m http.server). The POC show how an attacker can "inject" true into a cookie that suppose to be false without changing the cookie value.

This is more dangerous than it seems, considering Firefox uses a weak cookie system across dev tool profiles: if you have the same website opened both in your normal profile and in your "private browsing" mode. The cookies are not shared to the website, however, if you open the table in the private tab, and add cookie from the normal mode, you'll see it also in the "private browsing" table (once again, the cookies themselves don't share between the two, just the table).

Comment 1

[:Gijs (he/him)]

(In reply to matan.honig2 from comment #0)

This is more dangerous than it seems, considering Firefox uses a weak cookie system across dev tool profiles: if you have the same website opened both in your normal profile and in your "private browsing" mode. The cookies are not shared to the website, however, if you open the table in the private tab, and add cookie from the normal mode, you'll see it also in the "private browsing" table (once again, the cookies themselves don't share between the two, just the table).

This is known as bug 1640118.

Comment 2

[:Gijs (he/him)]

Attachment: Screenshot running the PoC

I can't really reproduce the original issue as described and am struggling to understand what you think is a security problem here (even if the storage inspector table is buggy).

Can you clarify?

Comment 3

[matan.honig2]

Yes. It's strange you can't reproduce the original issue with my POC. I screenshotted it, but I can't upload images here. on each OS I test on, the "true" was close to the "IsSecure" (so it looks like "IsSecure" was "true"). Maybe it's the font.

From a security viewpoint, it can be useful for the attacker to disable the ability of the user to detect and recognize unknown cookies. For example, if the attacker has an XSS on a website (or on a subdomain), and the victim enters it, but the attacker cannot control the http-only cookies, it may be useful for the attacker to mislead the user to make it seem like another cookie, e.g., a lot of sites use an http-only cookie like "YOUR_TOKEN_SECRET_PLEASE_DONT_SHARE", and the attacker can use some limited XSS on a subdomain to make the cookie name look more like "public_username", to bypass this level of protection. (The mention of the cookie bug in a private profile is to say that it can happen even if the user opens the vulnerable subdomain in a private profile)

Comment 4

[matan.honig2]

Attachment: cookies_screenshot.png

Comment 5

[Daniel Veditz [:dveditz]]

In your picture. and even in Gijs's, I see that the cookie values are taller than the other cells in the row and are increasingly shifted down. How much likely depends heavily on OS and font. Is that what you're reporting here? That won't affect normal users (they shouldn't be looking in here), and developers who are debugging their sites will likely notice all the extra cookies you had to spam in there to achieve this effect.

Comment 6

[Daniel Veditz [:dveditz]]

This does not need to be hidden. It's a rendering bug

Comment 7

[Julian Descottes [:jdescottes]]

We could try to fix a height in the class for the table cell in storage.css: https://searchfox.org/mozilla-central/source/devtools/client/themes/storage.css

This should fix the issue. Would be a good first bug, probably just a css fix, maybe add a small test to check that all cells have the same height even with special characters.

Comment 8

[anshumanjofficial]

Hi Nicolas, if this issue is not solved, can you please assign this to me?

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

(In reply to anshumanjofficial from comment #8)

Hi Nicolas, if this issue is not solved, can you please assign this to me?

sure, It's assigned to you now :)

Comment 10

[anshumanjofficial]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #9)

(In reply to anshumanjofficial from comment #8)

Hi Nicolas, if this issue is not solved, can you please assign this to me?

sure, It's assigned to you now :)

Thanks

Comment 11

[anshumanjofficial]

Hi Nicolas, as I am new contributor, I may need your guidance to solve it, can you please connect with me ?

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

(In reply to anshumanjofficial from comment #11)

Hi Nicolas, as I am new contributor, I may need your guidance to solve it, can you please connect with me ?

Hello, here's some documentation you can read to help you getting started: https://firefox-source-docs.mozilla.org/devtools/index.html

When working on DevTools (or Firefox UI in general), the Browser Toolbox (https://firefox-source-docs.mozilla.org/devtools-user/browser_toolbox/index.html) is extremely valuable so you can inspect DevTools with DevTools :)

This specific issue can probably be fixed in CSS, setting a fixed height (20px seems to be nice) for the cells. The stylesheet used for the storage panel is https://searchfox.org/mozilla-central/source/devtools/client/themes/storage.css.

Let me know if you have any question, here, or in https://chat.mozilla.org/#/room/#devtools:mozilla.org

Comment 13

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

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 15

[Chris Vander Linden]

I'm new here but I believe I have a simple solution for this that avoids just setting a fixed height to help the table from breaking if someone adjusts the font size. Could you assign it to me, if it's still open? Thank you so much!

Comment 16

[Chris Vander Linden]

Attachment: Bug 1888847 - Fix Storage Inspector table layout for dynamic content. r=#devtools

This patch addresses a layout issue in Firefox DevTools' storage inspector:

• Ensures dynamic table rows maintain a baseline height with min-height: 19.5px.
• Fixes alignment issues caused by tall special characters or large font sizes.
• Retains the original design while improving robustness for dynamic content.

Comment 17

[:Gijs (he/him)]

Chris, just a quick note to say thank you for the patch! Hopefully post-holiday season (end of this week or sometime next week) someone will be along to review this. :-)

Comment 18

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 19

[Chris Vander Linden]

Attachment: Bug 1888847 - Fix row height syncing logic. r=nchevobbe

Comment 20

[Chris Vander Linden]

Attachment: Bug 1888847 - Synchronize row heights in table widget to fix misalignment. r=nchevobbe

Comment 21

[Chris Vander Linden]

Attachment: Bug 1888847 - Sync row height in TableWidget.js

Comment 22

[Chris Vander Linden]

Attachment: Bug 1888847 - Sync row height for TableWidget to handle tall content and maintain alignment

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a0c98e624cb4
https://hg.mozilla.org/integration/autoland/rev/0d7424c100f5
Sync row height for TableWidget to handle tall content and maintain alignment r=nchevobbe,devtools-reviewers

Comment 24

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0d7424c100f5