richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 259 forks source link

upload attack surface - URI malformed error #342

Closed boly38 closed 9 months ago

boly38 commented 1 year ago

Hi, We are using express-fileupload on production public website, and encounter some regular attack attempts as well (as on many websites).

One of them is a strange POST against an unmapped api endpoint that cause unexpected error: URI malformed error

Error Full Stack Sample

2022-11-10T20:05:57: Error: Unexpected end of form - POST /logupload?logMetaData=%7B%22itrLogPath%22%3A%20%22..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fhttpd%2Fhtml%2Fwsgi_log_upload%22%2C%20%22logFileType%22%3A%20%22log_upload_wsgi.py%22%2C%20%22workloadID%22%3A%20%222%22%7D - 500
2022-11-10T20:06:16: /var/www/myproject/node_modules/express-fileupload/lib/utilities.js:232
2022-11-10T20:06:16:   return opts.uriDecodeFileNames ? decodeURIComponent(fileName) : fileName;
2022-11-10T20:06:16:                                    ^
2022-11-10T20:06:16:
2022-11-10T20:06:16: URIError: URI malformed
2022-11-10T20:06:16:     at decodeURIComponent (<anonymous>)
2022-11-10T20:06:16:     at uriDecodeFileName (/var/www/myproject/node_modules/express-fileupload/lib/utilities.js:232:36)
2022-11-10T20:06:16:     at parseFileName (/var/www/myproject/node_modules/express-fileupload/lib/utilities.js:281:16)
2022-11-10T20:06:16:     at Multipart.<anonymous> (/var/www/myproject/node_modules/express-fileupload/lib/processMultipart.js:52:22)
2022-11-10T20:06:16:     at Multipart.emit (node:events:390:28)
2022-11-10T20:06:16:     at HeaderParser.cb (/var/www/myproject/node_modules/busboy/lib/types/multipart.js:358:14)
2022-11-10T20:06:16:     at HeaderParser.push (/var/www/myproject/node_modules/busboy/lib/types/multipart.js:162:20)
2022-11-10T20:06:16:     at SBMH.ssCb [as _cb] (/var/www/myproject/node_modules/busboy/lib/types/multipart.js:394:37)
2022-11-10T20:06:16:     at feed (/var/www/myproject/node_modules/streamsearch/lib/sbmh.js:219:14)
2022-11-10T20:06:16:     at SBMH.push (/var/www/myproject/node_modules/streamsearch/lib/sbmh.js:104:16)

Pre analysis on how to reproduce

Following a quick search on that kind of decodeURIComponent error, I found that some encoded caracters in filename could produce this.

How To Reproduce

1) create a file having some characters in content, and the following name; bug_bounty_upload_%91and%92.txt 2) create a POST request (ex. using postman), on your express endpoint, example

POST http://localhost:4000/logupload?logMetaData=%7B%22itrLogPath%22%3A%20%22..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fhttpd%2Fhtml%2Fwsgi_log_upload%22%2C%20%22logFileType%22%3A%20%22log_upload_wsgi.py%22%2C%20%22workloadID%22%3A%20%222%22%7D

select a Body with form-data file that point on this file. image

This is cUrl equivalent

curl --location --request POST 'http://localhost:4000/logupload?logMetaData=%7B%22itrLogPath%22%3A%20%22..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fhttpd%2Fhtml%2Fwsgi_log_upload%22%2C%20%22logFileType%22%3A%20%22log_upload_wsgi.py%22%2C%20%22workloadID%22%3A%20%222%22%7D' \
--form 'file=@"/C:/TMP/bug_bounty_upload_%91and%92.txt"'

Exected behavior

I expect a fix or a way to avoid this error. (with an option to generate or not info log ?)

security question

If you reproduce this issue on your side, I think maintainer have to create a security advisory entry

what do you think ? Best Regards

RomanBurunkov commented 10 months ago

Hi @boly38 ,

Thanks for your suggestion and PR. I checked it and from the top of my head created the following code snippet:

function decode(input) {
  const matcher = /(%[a-f0-9]{2})/gi;

  return input.split(matcher)
    .map((str) => {
      try {
        return decodeURIComponent(str);
      } catch (err) {
        console.log(str, err.message);
        return '';
      }
    })
    .join('');
}

let str = 'bug_bounty_upload_%91and%92.txt';
let res = decode(str);
console.log(res);

The main idea is firstly try to decodeURIComponent the full string and then if it fails run the custom decode, which split the string with escaped chars and skip only those for which it gets error.

RomanBurunkov commented 9 months ago

Fixed with #356 in version 1.4.1