Formidable <3.2.4 Arbitrary File Upload Critical Severity

[domcorso-nib]

When running npm audit report we are seeing a critical vulnerability due to the version of formidable being used.

I've seen previous issues such as #1725 which indicate that it has been revoked but I can see it on the GitHub database last updated a few hours ago:
GHSA-8cp3-66vr-3r4c

Are there any plans to upgrade to the latest version of formidable?

[shivam178]

Same. Dependabot has raised a critical security alert for us. Formidable should be upgraded soon.

Dependabot cannot update formidable to a non-vulnerable version
The latest possible version that can be installed is 2.1.2 
The earliest fixed version is 3.2.4.

[lukewang2018]

It's the same issue for me. any plan to fix it? Thanks.

# npm audit report

formidable  <3.2.4
Severity: critical
Formidable arbitrary file upload - https://github.com/advisories/GHSA-8cp3-66vr-3r4c
No fix available
node_modules/formidable
  superagent  >=0.4.0
  Depends on vulnerable versions of formidable

[un4ckn0wl3z]

Same here.

[kasparfalkenberg]

There was a PR to update formidable to latest (3.5.1). Why was it closed?

[kudlav]

Older issue: #1781

[kasparfalkenberg]

@kudlav so it's the node js version issue? As the esm shouldn't be a problem anymore

[tomstrong64]

formidable v3 changed to ES modules, the breaking change is in the parser callback params. files now returns an array instead of an object regardless of how many files are uploaded. Writing a fix now
formidable v2

formidable v3

[Yehonal]

I'm using audit-ci in my pipeline and the npm audit fix --force is not able to fix this issue. So I was wondering: what do you guys use in this case if you have audit checks in your CI/CD? can the "overrides" option from the npm package.json be a solution or there's no better way than temporarily allow it in the audit-ci tool?

[tomstrong64]

Difference between test results before update and after update
Before:

After:

Just trying to figure out a safe and reliable way to map the new result format

[tomstrong64]

Working with http2 but getting timeouts with http1

[tomstrong64]

Remaining issues with http1 when using formidable v3:

• The user.txt test file seems to upload, but user.json and user.html don't seem to upload...
• Requests where request.post().field() calls timeout even when setting the timeout to 60 seconds

[tomstrong64]

Remaining issues with http1 when using formidable v3:

• The user.txt test file seems to upload, but user.json and user.html don't seem to upload...
• Requests where request.post().field() calls timeout even when setting the timeout to 60 seconds

Struggling to make any progress. Can someone pull my changes and have a look?
Pull request: #1800

[erwanriou]

the fix would be to clone this library, create your own with an updated formidable package version...

[Anoerak]

I'm using audit-ci in my pipeline and the npm audit fix --force is not able to fix this issue. So I was wondering: what do you guys use in this case if you have audit checks in your CI/CD? can the "overrides" option from the npm package.json be a solution or there's no better way than temporarily allow it in the audit-ci tool?

the npm override is a functional temporary fix if you're not using the formidable dep.

"overrides": {
    "superagent": {
        "formidable": "3.2.5"
    }
  },

[tomstrong64]

I'm using audit-ci in my pipeline and the npm audit fix --force is not able to fix this issue. So I was wondering: what do you guys use in this case if you have audit checks in your CI/CD? can the "overrides" option from the npm package.json be a solution or there's no better way than temporarily allow it in the audit-ci tool?

the npm override is a functional temporary fix if you're not using the formidable dep.

"overrides": {
    "superagent": {
        "formidable": "3.2.5"
    }
  },

I would like to warn you that multipart/form-data requests may not work as intended with this solution.

[quinnturner]

@Yehonal

I'm using audit-ci in my pipeline and the npm audit fix --force is not able to fix this issue. So I was wondering: what do you guys use in this case if you have audit checks in your CI/CD? can the "overrides" option from the npm package.json be a solution or there's no better way than temporarily allow it in the audit-ci tool?

I am the author of audit-ci. Using audit-ci, you can suppress the advisory by using the allowlist feature. https://github.com/ibm/audit-ci#allowlisting. In this case, it's best to provide the GitHub Advisory ID, which you can find in the Security tab in your GitHub project's page or in the audit-ci logs.

URL:
https://github.com/YOUR-ORG/YOUR-PROJECT/security/dependabot

From there, you can see the GitHub advisory ID. In this case, you can also find a link to the GitHub advisory itself.

You can view the advisory directly: GHSA-8cp3-66vr-3r4c

[modestaspruckus]

I'm using audit-ci in my pipeline and the npm audit fix --force is not able to fix this issue. So I was wondering: what do you guys use in this case if you have audit checks in your CI/CD? can the "overrides" option from the npm package.json be a solution or there's no better way than temporarily allow it in the audit-ci tool?

the npm override is a functional temporary fix if you're not using the formidable dep.

"overrides": {
    "superagent": {
        "formidable": "3.2.5"
    }
  },

If someone will face issue regarding ES module here is the config snippet for jest

"transform": {
      "\\.[jt]sx?$": "babel-jest"
},
"transformIgnorePatterns": [
      "node_modules/(?!(formidable)/)"
]

[domcorso-nib]

Thanks for the quick turnaround 🙏

[skubot]

Hey guys, we use a few packages that are still using v8 of superagent, ie: supertest and json-refs. I am not sure how soon they will be updated to v9... in the meantime would be possible to get a point update under v8? I know this issue is closed, should I create a new issue?

edit:
Sorry about this... actually could we get a patched v7 too?

supertest@6.3.4 uses superagent@8.1.2
json-refs@3.0.15 uses superagent@7.1.6

edit: I'm going to stop using json-refs that package is stale, so... nevermind regarding v7 thanks.

[GwenTL]

It looks like the GitHub Action test run fails silently on the same http1 tests as raised by @tomstrong64 above still. Is that not an indication v9.0.1 might still include an issue for http1 multipart?

[tomstrong64]

It looks like the GitHub Action test run fails silently on the same http1 tests as raised by @tomstrong64 above still. Is that not an indication v9.0.1 might still include an issue for http1 multipart?

Looking into this again now, I think the it may be an issue in formidable().parse() as it tends to hang on most of the requests with http1. Even given 20 seconds instead of the normal 5 seconds for the tests they still timeout, with the 3 file upload test being the exception, only the last file gets uploaded.

Console errors to see where the code is getting to:

Results are the same given 5 or 20 seconds:

Errors given at end of tests:

For 3) should attach a file swapping the order of the .attach() in the test shows it only gets the last attachment:

[tomstrong64]

Unfortunately I don't have time today to have a look into the changes in formidable, but for anyone who wants to investigate this, here's the code for the old and new versions of the parser (I couldn't find 2.1.2 but found 2.1.1):

2.1.1: https://github.com/node-formidable/formidable/blob/v2-latest/src/parsers/Multipart.js
3.5.1: https://github.com/node-formidable/formidable/blob/master/src/parsers/Multipart.js

[GwenTL]

Unfortunately I don't have time today to have a look into the changes in formidable, but for anyone who wants to investigate this, here's the code for the old and new versions of the parser (I couldn't find 2.1.2 but found 2.1.1):

2.1.1: https://github.com/node-formidable/formidable/blob/v2-latest/src/parsers/Multipart.js 3.5.1: https://github.com/node-formidable/formidable/blob/master/src/parsers/Multipart.js

I'm having more of a look again, but also will have to stop at some point. 🤞

[tomstrong64]

I couldn't help myself, but to have a quick look haha, I will hopefully come back to it later if no one has figured it out.

But adding console.error() to the start of every method in the MultipartParser class in node_modules/formidable/dist/index.cjs gave me some interesting results.

Example of the failing tests with http1:

Example of passing tests with http2:

[devkan]

Same here...
Isn't there a simple workaround?
I love nest.js, but this is so hard.

[tomstrong64]

Just another quick update.
The one's timing out don't seem to ever trigger the parser.on('data') event handler.

node_modules/formidable/dist/index.cjs

[tomstrong64]

In this code snippet below, in http1 self.req.req is defined, but in http2 it is undefined. No idea if this will help at all, but the fact that the self object has differences in its format for the request object raises questions for me.

node_modules/formidable/dist/index.cjs

Here is also a comparison of the headers:
http1

http2

Hope this info helps!

[tomstrong64]

I am more clueless than I was before, I haven't made any code changes but ran the tests again and this time the second failing test received the "data" event and got the data from the last field attached... request_.field('user[species]', 'ferret');

[bnorb]

The advisory was withdrawn again