CVE-2023-45367: ⚓ T344923 User can store arbitrary number of rows in cu_useragent_clienthints

**

User can store arbitrary number of rows in cu_useragent_clienthints

Closed, ResolvedPublicSecurity

**

• Edit Task

• Edit Related Tasks…

• Edit Related Objects…

• Mute Notifications

• Protect as security issue

• Award Token

• Flag For Later

When sending client hints data, the brands and fullVersionList fields are a list. Each member of these lists is inserted as a separate row in the cu_useragent_clienthints table.

There does not seem to be a limit to the number of items a user can put in these lists. I was able to submit a client hints request with 10000 entries in both of these fields, and 20000 rows were inserted in to the database.

This could be a possible vector for denial of service.

Reproduction

1. Setup a wiki with CheckUser and enable client hints (they are enabled by default, you shouldn’t need to do anything)
2. Make an edit in Firefox (which does not support client hints)
3. Find out the revision ID of the edit you just made
4. Run this command, changing <rev id> to the revision ID you just found. You might need to change the address of the server as well:

curl ‘http://localhost:8080/w/rest.php/checkuser/v0/useragent-clienthints/revision/<rev id>’ -H ‘Content-Type: application/json’ \ –data-raw ‘{"architecture":"","bitness":"64","brands":[{"brand": "Test Brand", "version": "0"}, {"brand": "Test Brand", "version": "1"}, {"brand": "Test Brand", "version": "2"}, {"brand": "Test Brand", "version": "3"}, {"brand": "Test Brand", "version": "4"}, {"brand": "Test Brand", "version": "5"}, {"brand": "Test Brand", "version": "6"}, {"brand": "Test Brand", "version": "7"}, {"brand": "Test Brand", "version": "8"}, {"brand": "Test Brand", "version": "9"}],"fullVersionList":[{"brand": "Test Version", "version": "0"}, {"brand": "Test Version", "version": "1"}, {"brand": "Test Version", "version": "2"}, {"brand": "Test Version", "version": "3"}, {"brand": "Test Version", "version": "4"}, {"brand": "Test Version", "version": "5"}, {"brand": "Test Version", "version": "6"}, {"brand": "Test Version", "version": "7"}, {"brand": "Test Version", "version": "8"}, {"brand": "Test Version", "version": "9"}],"mobile":false,"model":"","platform":"Linux","platformVersion":"5.10.0"}’

5. Go to the database and run SELECT * FROM cu_useragent_clienthints;

Other information

Only wmf and the master branch of CheckUser has this issue. There will be no need to keep this private after the patch is in production as only local testing wikis should have this issue until it’s merged into the master branch.

Risk Rating

Medium

Author Affiliation

WMF Product

• Mentions

Event Timeline

Dreamy_Jazz triaged this task as High priority.

Comment Actions

I will write a patch.

There are a few ways to get this fixed that I can see:

1. Disable collection of Client Hints globally until this is fixed (and then a fix can be written normally)
2. Temporarily disable collection, deploy a security patch and then re-enable collection.
3. Deploy a security patch without disabling collection

Comment Actions

Proposed patch:

This patch includes the fix and also adding a new testcase to an existing test to check that the fix worked. I’ve tested this locally and this seems to fix the issue.

Comment Actions

Proposed patch:

This patch includes the fix and also adding a new testcase to an existing test to check that the fix worked. I’ve tested this locally and this seems to fix the issue.

virtual +2 from me. @Dreamy_Jazz is making some minor changes to the commit message.

Comment Actions

Modified proposed patch:

Changes were to fix the commit message and to add an inline comment to the array_splice call on the suggestion of Kosta.

Comment Actions

Modified proposed patch:

Changes were to fix the commit message and to add an inline comment to the array_splice call on the suggestion of Kosta.

+2, thank you.

Comment Actions

@thcipriani just synced this.

Is there a SAL entry for this? I couldn’t find an obvious one. Also now tracking at T276237 and T340874.

Comment Actions

@thcipriani just synced this.

Is there a SAL entry for this? I couldn’t find an obvious one. Also now tracking at T276237 and T340874.

No, I don’t see one.

@sbassett are we OK to proceed with creating a patch in Gerrit for this, and making this task public?

Comment Actions

@sbassett are we OK to proceed with creating a patch in Gerrit for this, and making this task public?

Yes, that’s fine. Once an issue is patched in Wikimedia production and if it doesn’t belong to a bundled component, then it can be disclosed and backported. We’ll disclose it again via the end-of-quarter supplemental security release as well (T340874).

Comment Actions

@sbassett are we OK to proceed with creating a patch in Gerrit for this, and making this task public?

Yes, that’s fine. Once an issue is patched in Wikimedia production and if it doesn’t belong to a bundled component, then it can be disclosed and backported. We’ll disclose it again via the end-of-quarter supplemental security release as well (T340874).

The vulnerability has only existed in the master and wmf branches, so not sure if it needs to be backported when making the fix public as no release versions included it. This may also mean that it doesn’t need to be stated in the end-of-quarter supplemental security release as the fix would not be included in any of these releases (due to the vulnerability not existing in any release version).

Comment Actions

The vulnerability has only existed in the master and wmf branches, so not sure if it needs to be backported when making the fix public as no release versions included it. This may also mean that it doesn’t need to be stated in the end-of-quarter supplemental security release as the fix would not be included in any of these releases (due to the vulnerability not existing in any release version).

So at the very least there should be a public backport to master or main for any security patch, which I see you’ve now done in c952482. If the patch is not relevant for other supported release branches, then no further effort needs to be expended in backporting it to said branches. I’ll go ahead and make this task public now as well.

sbassett changed Risk Rating from N/A to Medium.

Comment Actions

I had assumed that the name “backport” applied only to patches not on the master branch. My intended meaning was that it didn’t need to be cherry picked to other branches from the master branch.

Comment Actions

Fix is deployed on production, but waiting for code review on the master branch.

Comment Actions

/srv/patches/1.41.0-wmf.24/extensions/CheckUser/01-T344923-2.patch fails to apply as of Tue, 29 Aug 2023 00:55:16 UTC.

Comment Actions

/srv/patches/1.41.0-wmf.24/extensions/CheckUser/01-T344923-2.patch fails to apply as of Tue, 29 Aug 2023 00:55:16 UTC.

This has been merged into the master branch, so should be in wmf.24. As such I presume it is safe to ignore this for wmf.24.

Comment Actions

/srv/patches/1.41.0-wmf.24/extensions/CheckUser/01-T344923-2.patch fails to apply as of Tue, 29 Aug 2023 00:55:16 UTC.

Do we still need this patch? @thcipriani sycned it on August 24 (T344923#9118791) and the master branch patch merged on August 26 (T344923#9121451).

Comment Actions

Sounds like we don’t need the patch anymore so I removed it.

Comment Actions

I have only been able to store up to 10 rows each for brands and fullVersionList in the cu_useragent_clienthints table.

Test environment: local docker mysql CheckUser 2.5 (dfa9b11) 20:20, 4 September 2023.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL