| MoritzMuehlenhoff | |
| Jan 13 2023, 11:30 AM |
| F36511831: patch.diff | |
| Jan 26 2023, 2:30 PM |
(Note it's a CVE-2020 ID, but it was only published in 2023):
VisualEditor bundles a copy of PapaParse (lib/ve/lib/papaparse/papaparse.js).
There was a report about a ReDoS vulnerability for it: https://github.com/mholt/PapaParse/issues/777
The fix is https://github.com/mholt/PapaParse/commit/235a12758cd77266d2e98fd715f53536b34ad621
We should include the patch (or upgrade to 5.3.0, didn't study the other changes between 5.1.0 and 5.3.0)
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Reedy | T325839 Release MediaWiki 1.35.10/1.38.6/1.39.3 | |||
| Resolved | Reedy | T325840 Tracking bug for MediaWiki 1.35.10/1.38.6/1.39.3 | |||
| Resolved | Security | Esanders | T326946 CVE-2020-36649: Bundled PapaParse copy in VisualEditor has known ReDos |
Everything appears to work fine with an upgrade to 5.3.2. Do I need to prepare it as a security patch, or just submit it normally?
Does that basically just involve building the new version for VE's lib directory and maybe updating some READMEs? If so, I would think that could just go through gerrit and maybe get picked to 1.40.0-wmf.19 today? Unless @Reedy et al would prefer we keep this a bit more secure and get it deployed via a security patch and officially track it for the next security release.
How/where does VE actually use this code? Is it (trivially/unlikely/impossible) to execute code in the PapaParse library that would be run through that regex?
Just helps gauge the potential way of going around it.
For example, if VE never executes that code (which can happen with larger libraries and dependancies), it's could be done as a trivial lib bump in Gerrit.
But if it's a critical code path in VE, and it's used everywhere all the time, we might want to be more cautious.
As it's a CSV parser, I'm guessing it's probably somewhere in between. Likely not executed much, but it would be possible to exploit the ReDos as the code can be/is used when CSV stuff is in play?
Looks like we're using the same version (at least) back to REL1_35, so the backport should be relatively trivial (though submodule bumps etc make it a little more involved... But there's scripts in the repos to help).
I think it's only used in ve.ui.DSVFileTransferHandler, so it'll only trigger when an upload actually happens. I.e. you'd actually need to get someone to drag-drop a malicious file onto a VE window before this could be exploited.
Yes, if you drag a drop a CSV file into VE it will convert it into a table automatically.
In this case we don't really have any security impact, right? This essentially means that if someone uploads a botched CSV, they'd slow down their own browser tab/VE session for a bit, but nothing more.
It's very low-risk, for sure. I'd personally be fine with it going through gerrit unless anyone has serious objections.
Doing it in public on Gerrit sounds okay to me.
Can we make this task public as well?
I can't see why not...
Would be good to note in the commit summary the CVE number (for traceability), along with why we consider it low risk (so people don't have to read the task for info).
Change 884839 had a related patch set uploaded (by Esanders; author: Esanders):
[VisualEditor/VisualEditor@master] Update papaparse to 5.3.2
Change 884839 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Update papaparse to 5.3.2
Change 884925 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (6ed9b00f1)
Change 884505 had a related patch set uploaded (by Reedy; author: Esanders):
[VisualEditor/VisualEditor@REL1_39] Update papaparse to 5.3.2
Change 884926 had a related patch set uploaded (by Reedy; author: Esanders):
[VisualEditor/VisualEditor@REL1_38] Update papaparse to 5.3.2
Change 884927 had a related patch set uploaded (by Reedy; author: Esanders):
[VisualEditor/VisualEditor@REL1_35] Update papaparse to 5.3.2
Change 884505 merged by jenkins-bot:
[VisualEditor/VisualEditor@REL1_39] Update papaparse to 5.3.2
Change 884926 merged by jenkins-bot:
[VisualEditor/VisualEditor@REL1_38] Update papaparse to 5.3.2
Change 884927 merged by jenkins-bot:
[VisualEditor/VisualEditor@REL1_35] Update papaparse to 5.3.2
Change 884925 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (6ed9b00f1)
Change 884537 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_35
Change 884538 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_38
Change 884539 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_39
Change 884540 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@REL1_39] Update VE core submodule to origin/REL1_39
Change 884539 abandoned by Reedy:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_39
Reason:
Change 884541 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@REL1_38] Update VE core submodule to origin/REL1_38
Change 884538 abandoned by Reedy:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_38
Reason:
Change 884542 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/VisualEditor@REL1_35] Update VE core submodule to origin/REL1_35
Change 884537 abandoned by Reedy:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to origin/REL1_35
Reason:
Change 884541 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_38] Update VE core submodule to origin/REL1_38
Change 884540 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_39] Update VE core submodule to origin/REL1_39
Change 884542 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_35] Update VE core submodule to origin/REL1_35