Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible Critical CVE - Multer file upload lib #9489

Closed
3 of 15 tasks
z3xddd opened this issue Apr 22, 2022 · 20 comments
Closed
3 of 15 tasks

Possible Critical CVE - Multer file upload lib #9489

z3xddd opened this issue Apr 22, 2022 · 20 comments
Labels
needs triage This issue has not been looked into

Comments

@z3xddd
Copy link

z3xddd commented Apr 22, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

Hi guys, I hope you are doing well!

I sent an email to support regarding this CVE found but I didn't get a response, so I'm opening this issue.

We identified a critical security issue when using the Multer lib to parse file uploads (FileInterceptor).

We have a web application using NestJS. During PenTest, when i send a multipart form data http request (file upload) and change the value of the Content-Type to "application/vdn.hzn-3d-crossword" our backend crashes with the following error:
node:events:504
throw er; // Unhandled 'error' event
^
Error: Unexpected end of multipart data
at /home/application/node_modules/dicer/lib/Dicer.js:62:28
at processTicksAndRejections (node:internal/process/task_queues:78:11)
Emitted 'error' event on Busboy instance at:
at Busboy.emit (/home/application/node_modules/busboy/lib/main.js:38:33)
at Dicer. (/home/application/node_modules/busboy/lib/types/multipart.js:281:9)
at Dicer.emit (node:events:526:28)
at Dicer.emit (/home/application/node_modules/dicer/lib/Dicer.js:80:35)
at /home/application/node_modules/dicer/lib/Dicer.js:62:14
at processTicksAndRejections (node:internal/process/task_queues:78:11)

Here is the request payload we used to reproduce the issue:
------WebKitFormBoundaryeQ3kzkwETmKP36OH
Content-Disposition: form-data; name="type"

DOCUMENTO_FRENTE
------WebKitFormBoundaryeQ3kzkwETmKP36OH
Content-Disposition: form-data; name="file"; filename="xss-2.jpg"
Content-Type: application/vdn.hzn-3d-crossword <-- Inject Payload

------WebKitFormBoundaryeQ3kzkwETmKP36OH--

We believe this case is a new CVE found in the multer lib or its dependencies which NestJS uses.

Here's a print of the AKS pods crashing when exploiting the vulnerability:
img1

An issue about this was opened directly in Multer and it was commented that this error had been solved, however it still happens in NestJS, indicating it is possibly a new CVE.

Minimum reproduction code

Sorry, for business policies we dont provide this code. To reproduce this issue is necessary web application with file upload function and use Multer lib to file upload parse.

Steps to reproduce

  1. Create web application with file upload function;
  2. Use Multer lib for parse file uploads (FileInterceptor);
  3. On file upload request, change Content-Type param for the payload: application/vdn.hzn-3d-crossword;
  4. Server down :)

Expected behavior

The lib shouldn't return an error and bring down the application's services. The correct thing would be to inform that this mimetype is not allowed. (This was configured in the application, but as the error occurs directly in the lib used, the treatment performed does not work).

Package

  • I don't know. Or some 3rd-party package
  • @nestjs/common
  • @nestjs/core
  • @nestjs/microservices
  • @nestjs/platform-express
  • @nestjs/platform-fastify
  • @nestjs/platform-socket.io
  • @nestjs/platform-ws
  • @nestjs/testing
  • @nestjs/websockets
  • Other (see below)

Other package

No response

NestJS version

8.4.4

Packages versions

{
  "name": "onboarding-edge-bff",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "prebuild": "rimraf dist",
    "prebuild:prod": "rimraf dist",
    "build": "nest build",
    "build:prod": "nest build --path ./tsconfig.prod.json",
    "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
    "start": "nest start -e 'node -r ./dist/opentelemetry.js'",
    "start:dev": "nest start --watch -e 'node -r ./dist/opentelemetry.js'",
    "start:debug": "nest start --debug=0.0.0.0 --watch -e 'node -r ./dist/opentelemetry.js'",
    "start:prod": "node -r './dist/opentelemetry.js' dist/main",
    "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
    "test": "jest --runInBand",
    "test:watch": "node --inspect-brk=0.0.0.0 -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --watch --runInBand",
    "test:ci": "jest --ci --testResultsProcessor=\"jest-junit\" --coverage --coverageReporters=cobertura",
    "test:cov": "jest --coverage -i",
    "test:debug": "node --inspect-brk=0.0.0.0 -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "ts-knex": "ts-node -r dotenv/config -r tsconfig-paths/register node_modules/.bin/knex",
    "knex": "node -r dotenv/config node_modules/.bin/knex"
  },
  "dependencies": {
    "@nestjs/axios": "^0.0.7",
    "@nestjs/class-transformer": "^0.4.0",
    "@nestjs/class-validator": "^0.13.4",
    "@nestjs/common": "^8.4.4",
    "@nestjs/config": "^2.0.0",
    "@nestjs/core": "^8.4.4",
    "@nestjs/jwt": "^8.0.0",
    "@nestjs/microservices": "^8.4.4",
    "@nestjs/platform-express": "^8.4.4",
    "@nestjs/platform-socket.io": "^8.4.4",
    "@nestjs/throttler": "^2.0.1",
    "@nestjs/websockets": "^8.4.4",
    "@opentelemetry/api": "^1.0.4",
    "@opentelemetry/core": "^1.0.1",
    "@opentelemetry/exporter-zipkin": "^1.0.1",
    "@opentelemetry/instrumentation": "^0.27.0",
    "@opentelemetry/instrumentation-http": "^0.27.0",
    "@opentelemetry/instrumentation-knex": "^0.27.1",
    "@opentelemetry/instrumentation-winston": "^0.27.3",
    "@opentelemetry/propagator-b3": "^1.0.1",
    "@opentelemetry/resources": "^1.0.1",
    "@opentelemetry/sdk-node": "^0.27.0",
    "@opentelemetry/sdk-trace-base": "^1.0.1",
    "@opentelemetry/sdk-trace-node": "^1.0.1",
    "@opentelemetry/semantic-conventions": "^1.0.1",
    "@socket.io/postgres-adapter": "^0.2.0",
    "axios": "^0.26.1",
    "cookie-parser": "^1.4.6",
    "csurf": "^1.11.0",
    "dotenv": "^16.0.0",
    "express": "^4.17.3",
    "form-data": "^4.0.0",
    "frameguard": "^4.0.0",
    "kafkajs": "1.15.0",
    "knex": "^1.0.5",
    "nest-winston": "^1.6.2",
    "opentelemetry-instrumentation-kafkajs": "^0.27.0",
    "opentelemetry-instrumentation-socket.io": "^0.27.0",
    "pg": "^8.7.3",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.5.5",
    "socket.io": "^4.4.1",
    "uuid": "^8.3.2",
    "winston": "^3.7.2"
  },
  "devDependencies": {
    "@commitlint/cli": "^16.2.3",
    "@commitlint/config-conventional": "^16.2.1",
    "@nestjs/cli": "^8.2.5",
    "@nestjs/schematics": "^8.0.10",
    "@nestjs/testing": "^8.4.4",
    "@types/chai": "^4.3.0",
    "@types/common-tags": "^1.8.1",
    "@types/cookie-parser": "^1.4.2",
    "@types/csurf": "^1.11.2",
    "@types/dompurify": "^2.3.3",
    "@types/express": "^4.17.13",
    "@types/jest": "^27.4.1",
    "@types/lodash": "^4.14.181",
    "@types/multer": "^1.4.7",
    "@types/node": "16.11.7",
    "@types/sinon": "^10.0.11",
    "@types/supertest": "^2.0.12",
    "@types/uuid": "^8.3.4",
    "@types/validator": "^13.7.2",
    "@typescript-eslint/eslint-plugin": "^5.18.0",
    "@typescript-eslint/parser": "^5.18.0",
    "chai": "^4.3.6",
    "eslint": "^8.12.0",
    "eslint-config-airbnb-base": "^15.0.0",
    "eslint-config-airbnb-typescript": "^17.0.0",
    "eslint-config-prettier": "^8.5.0",
    "eslint-plugin-import": "^2.26.0",
    "eslint-plugin-prettier": "^4.0.0",
    "husky": "^7.0.4",
    "jest": "^27.5.1",
    "jest-junit": "^13.1.0",
    "prettier": "^2.6.2",
    "sinon": "^13.0.1",
    "supertest": "^6.2.2",
    "ts-jest": "^27.1.4",
    "ts-loader": "^9.2.8",
    "ts-node": "^10.7.0",
    "tsconfig-paths": "^3.14.1",
    "typescript": "^4.6.3"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": ".",
    "testRegex": ".*\\.spec\\.ts$",
    "testPathIgnorePatterns": [
      ".*\\e2e\\.spec\\.ts$"
    ],
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "./src/**/*.(t|j)s"
    ],
    "coverageDirectory": "<rootDir>/coverage",
    "coveragePathIgnorePatterns": [
      "/node_modules/",
      "/database/",
      "/config/",
      ".module.ts",
      "<rootDir>/src/main.ts",
      "/swagger/",
      ".env.ts",
      ".enum.ts",
      "test/",
      "/scripts/"
    ],
    "testEnvironment": "node",
    "testResultsProcessor": "jest-junit"
  }
}

Node.js version

16 Alpine

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

The web application run on Kubernetes cluster using docker image.

@z3xddd z3xddd added the needs triage This issue has not been looked into label Apr 22, 2022
@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

Could you please provide a minimum reproduction repository so that we can verify this ourselves? It might be as simple as updating the multer version but we'd need something to test against with reproduction steps

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

Hello @jmcdo29!!

I work as a Pentester, I'll talk to the dev team to create something specific for this CVE and I'll forward the link to you.

Thanks in advance for your help.

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

Here is the code for this CVE: https://github.com/z3xddd/Repository-for-test-CVE-NestJS

If the CVE is accepted, please let me know so we can start creating it.

@mbuogo
Copy link

mbuogo commented Apr 22, 2022

Top man!

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

Okay, so what do I need to do to reproduce this? Sending a file from the index.html seems to work fine. I also tried using the curl

curl -X POST http://localhost:3000 -F "[email protected];type=application/vdn.hzn-3d-crossword" -F 'name=test'

And got a successful return with the ability to send more requests afterwards. Is there something I should be doing instead?

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

Hello @jmcdo29.

Here is the movie PoC: https://we.tl/t-CjWwHPcIGb

  1. npm i
  2. npm build
  3. npm run start
  4. Open Firefox
  5. Open local web server
  6. Send one image file with no alteration;
  7. Send again the same image and inject payload on Content-Type: application/vnd.hzn-3d-crossword

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

Send again the same image and inject payload on Content-Type: application/vnd.hzn-3d-crossword

Can you give steps on how to do this specifically? Most everything I do is from terminal/command line so I'm not sure what I would need to do to inject that payload

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

Okay.

First:
npm i
npm build
npm run start

To exploit vulnerability:

  1. Open Firefox.
  2. Open url with your local web server;
  3. Open console tools and change for Network tab;
  4. Upload a image file;
  5. In console tools, click on request with you have uploaded file and use the options "Edit and Resend";
  6. Now its time to change value of Content-Type "image/png" to my payload "application/vnd.hzn-3d-crossword";
  7. Server down :)

Feel free to contact me if you have questions about this.

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

In the moovie PoC it's all step by step.

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

Okay, I did try that, got a 500 response, but the server was not taken down and I could still send requests.

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

In my case the server not still send request and on my Kubernetes Cloud Server the same.

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

You see the moovie Poc?

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

Well, with the reproduction provided, while the server sends back a 500 it does not crash, so I don't believe this needs to be reported as a CVE, correct?

You see the moovie Poc?

Yes I did. I saw that you got a 500 back because of an invalid Content-Type, but saw no signs of the server crashing

@jmcdo29 jmcdo29 closed this as completed Apr 22, 2022
@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

I do not agree, because in our tests the environment falls and is the same as we sent it.

Here is the first issue opened on Multer with a user with the same case: expressjs/multer#224

For me it remains a case of CVE yes.

Please review this case.

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

In the reproduction you provided the server

  1. responds with a 500 error code
  2. does not crash
  3. continues to receive requests

You wanted to report the CVE because the server would no longer accept requests, correct? Or am I missing something here? What is it that you are expecting should happen instead of the current behavior in the reproduction?

Also, the current version of multer being used is 1.4.2, so this should have been solved long ago.

@z3xddd
Copy link
Author

z3xddd commented Apr 22, 2022

In Kubernetes Server the service does crash.

Here's a print of the AKS pods crashing when exploiting the vulnerability:
image1

As you can see in this issue, it was commented in 2020 that this issue continues to be sustained in NestJS.
expressjs/multer#224

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

And I am telling you that with the reproduction you've provided this crash doesn't happen, so it's an issue in your code not our framework. If we cannot produce this outside of your server, then it's not a problem with the framework.

@jmcdo29
Copy link
Member

jmcdo29 commented Apr 22, 2022

If you truly believe this is a vulnerability we need concrete proof with a reproduction that has exact steps to take on how to make the crash happen when uploading a file like this. curl, xh, and using the file uploader in the steps you provided above caused an error but not a crash. Requests could still be accepted and the server still properly responded. Screenshots that show your server or k8s cluster or whatever crashed aren't enough, because we, the maintainers, cannot be certain you have the latest code, versions, etc. I'll be happy to take a look at this again if you can provide a reproduction of the aforementioned crash, but with the current reproduction, there's nothing that requires a CVE, in my opinion.

@micalevisk
Copy link
Member

micalevisk commented Apr 22, 2022

@z3xddd Using this project and following these steps, the server doesn't crashes here in my Ubuntu, just got TypeError: Cannot read properties of undefined (reading 'buffer') on terminal and 500 as a response. I also tried with node instead of nest start

Also, as this is related with FileInterceptor that do nothing but calling multer.single there's probably nothing we could do here 🤔

@z3xddd
Copy link
Author

z3xddd commented Apr 25, 2022

Hi guys @micalevisk @jmcdo29!

I'll pass it on to the dev team to check the codes again.

Thanks for the help and I apologize for the inconvenience.

@nestjs nestjs locked and limited conversation to collaborators May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

4 participants