From d027891cec67e190403fc4fa73f17d7a74f02720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Lang?= Date: Sun, 13 Jan 2019 23:27:43 -0300 Subject: Fix command injection when exporting to curl The command generated by `export.clip curl @focus` or `export.file curl @focus /path/to/file` wasn't being properly escaped so it could contain a malicious command instead of just a simple curl. --- mitmproxy/addons/export.py | 25 ++++++++++++++++--------- test/mitmproxy/addons/test_export.py | 28 +++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 90e95d3e..271fc49d 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -1,4 +1,5 @@ import typing +import shlex from mitmproxy import ctx from mitmproxy import command @@ -18,20 +19,26 @@ def raise_if_missing_request(f: flow.Flow) -> None: def curl_command(f: flow.Flow) -> str: raise_if_missing_request(f) - data = "curl " + args = ["curl"] request = f.request.copy() # type: ignore request.decode(strict=False) for k, v in request.headers.items(multi=True): - data += "-H '%s:%s' " % (k, v) + args += ["-H", shlex.quote("%s:%s" % (k, v))] if request.method != "GET": - data += "-X %s " % request.method - data += "'%s'" % request.url + args += ["-X", shlex.quote(request.method)] + args.append(shlex.quote(request.url)) if request.content: - data += " --data-binary '%s'" % strutils.bytes_to_escaped_str( - request.content, - escape_single_quotes=True - ) - return data + try: + content = strutils.always_str(request.content) + except UnicodeDecodeError: + # shlex.quote doesn't support a bytes object + # see https://github.com/python/cpython/pull/10871 + raise exceptions.CommandError("Request content must be valid unicode") + args += [ + "--data-binary", + shlex.quote(strutils.always_str(request.content)) + ] + return ' '.join(args) def httpie_command(f: flow.Flow) -> str: diff --git a/test/mitmproxy/addons/test_export.py b/test/mitmproxy/addons/test_export.py index f4bb0f64..5c365135 100644 --- a/test/mitmproxy/addons/test_export.py +++ b/test/mitmproxy/addons/test_export.py @@ -1,4 +1,5 @@ import os +import shlex import pytest import pyperclip @@ -47,23 +48,40 @@ def tcp_flow(): class TestExportCurlCommand: def test_get(self, get_request): - result = """curl -H 'header:qvalue' -H 'content-length:0' 'http://address:22/path?a=foo&a=bar&b=baz'""" + result = """curl -H header:qvalue -H content-length:0 'http://address:22/path?a=foo&a=bar&b=baz'""" assert export.curl_command(get_request) == result def test_post(self, post_request): - result = "curl -H 'content-length:256' -X POST 'http://address:22/path' --data-binary '{}'".format( - str(bytes(range(256)))[2:-1] - ) + post_request.request.content = b'nobinarysupport' + result = "curl -H content-length:15 -X POST http://address:22/path --data-binary nobinarysupport" assert export.curl_command(post_request) == result + def test_fails_with_binary_data(self, post_request): + # shlex.quote doesn't support a bytes object + # see https://github.com/python/cpython/pull/10871 + with pytest.raises(exceptions.CommandError): + export.curl_command(post_request) + def test_patch(self, patch_request): - result = """curl -H 'header:qvalue' -H 'content-length:7' -X PATCH 'http://address:22/path?query=param' --data-binary 'content'""" + result = """curl -H header:qvalue -H content-length:7 -X PATCH 'http://address:22/path?query=param' --data-binary content""" assert export.curl_command(patch_request) == result def test_tcp(self, tcp_flow): with pytest.raises(exceptions.CommandError): export.curl_command(tcp_flow) + def test_escape_single_quotes_in_body(self): + request = tflow.tflow( + req=tutils.treq( + method=b'POST', + headers=(), + content=b"'&#" + ) + ) + command = export.curl_command(request) + assert shlex.split(command)[-2] == '--data-binary' + assert shlex.split(command)[-1] == "'&#" + class TestExportHttpieCommand: def test_get(self, get_request): -- cgit v1.2.3 From eab4174b87c7ba0b7dab2c8d7e0b13253833abe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Lang?= Date: Sun, 13 Jan 2019 23:45:28 -0300 Subject: Fix command injection when exporting to httpie The command generated by `export.clip httpie @focus` or `export.file httpie @focus /path/to/file` wasn't being properly escaped so it could contain a malicious command instead of just a simple httpie call. --- mitmproxy/addons/export.py | 20 ++++++++++++-------- test/mitmproxy/addons/test_export.py | 27 ++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 271fc49d..761b3915 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -44,17 +44,21 @@ def curl_command(f: flow.Flow) -> str: def httpie_command(f: flow.Flow) -> str: raise_if_missing_request(f) request = f.request.copy() # type: ignore - data = "http %s " % request.method + args = ["http"] + args.append(shlex.quote(request.method)) request.decode(strict=False) - data += "%s" % request.url + args.append(shlex.quote(request.url)) for k, v in request.headers.items(multi=True): - data += " '%s:%s'" % (k, v) + args.append(shlex.quote("%s:%s" % (k, v))) if request.content: - data += " <<< '%s'" % strutils.bytes_to_escaped_str( - request.content, - escape_single_quotes=True - ) - return data + try: + content = strutils.always_str(request.content) + except UnicodeDecodeError: + # shlex.quote doesn't support a bytes object + # see https://github.com/python/cpython/pull/10871 + raise exceptions.CommandError("Request content must be valid unicode") + args += ["<<<", shlex.quote(content)] + return ' '.join(args) def raw(f: flow.Flow) -> bytes: diff --git a/test/mitmproxy/addons/test_export.py b/test/mitmproxy/addons/test_export.py index 5c365135..67d0fc99 100644 --- a/test/mitmproxy/addons/test_export.py +++ b/test/mitmproxy/addons/test_export.py @@ -85,23 +85,40 @@ class TestExportCurlCommand: class TestExportHttpieCommand: def test_get(self, get_request): - result = """http GET http://address:22/path?a=foo&a=bar&b=baz 'header:qvalue' 'content-length:0'""" + result = """http GET 'http://address:22/path?a=foo&a=bar&b=baz' header:qvalue content-length:0""" assert export.httpie_command(get_request) == result def test_post(self, post_request): - result = "http POST http://address:22/path 'content-length:256' <<< '{}'".format( - str(bytes(range(256)))[2:-1] - ) + post_request.request.content = b'nobinarysupport' + result = "http POST http://address:22/path content-length:15 <<< nobinarysupport" assert export.httpie_command(post_request) == result + def test_fails_with_binary_data(self, post_request): + # shlex.quote doesn't support a bytes object + # see https://github.com/python/cpython/pull/10871 + with pytest.raises(exceptions.CommandError): + export.httpie_command(post_request) + def test_patch(self, patch_request): - result = """http PATCH http://address:22/path?query=param 'header:qvalue' 'content-length:7' <<< 'content'""" + result = """http PATCH 'http://address:22/path?query=param' header:qvalue content-length:7 <<< content""" assert export.httpie_command(patch_request) == result def test_tcp(self, tcp_flow): with pytest.raises(exceptions.CommandError): export.httpie_command(tcp_flow) + def test_escape_single_quotes_in_body(self): + request = tflow.tflow( + req=tutils.treq( + method=b'POST', + headers=(), + content=b"'&#" + ) + ) + command = export.httpie_command(request) + assert shlex.split(command)[-2] == '<<<' + assert shlex.split(command)[-1] == "'&#" + class TestRaw: def test_get(self, get_request): -- cgit v1.2.3 From 674f92a7c164c538047c970fc42194211856276a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Lang?= Date: Sun, 13 Jan 2019 23:53:51 -0300 Subject: Refactor curl_command and httpie_command To avoid calling to shlex.quote many times --- mitmproxy/addons/export.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 761b3915..04c0349a 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -23,10 +23,10 @@ def curl_command(f: flow.Flow) -> str: request = f.request.copy() # type: ignore request.decode(strict=False) for k, v in request.headers.items(multi=True): - args += ["-H", shlex.quote("%s:%s" % (k, v))] + args += ["-H", "%s:%s" % (k, v)] if request.method != "GET": - args += ["-X", shlex.quote(request.method)] - args.append(shlex.quote(request.url)) + args += ["-X", request.method] + args.append(request.url) if request.content: try: content = strutils.always_str(request.content) @@ -36,20 +36,19 @@ def curl_command(f: flow.Flow) -> str: raise exceptions.CommandError("Request content must be valid unicode") args += [ "--data-binary", - shlex.quote(strutils.always_str(request.content)) + strutils.always_str(request.content) ] - return ' '.join(args) + return ' '.join(shlex.quote(arg) for arg in args) def httpie_command(f: flow.Flow) -> str: raise_if_missing_request(f) request = f.request.copy() # type: ignore - args = ["http"] - args.append(shlex.quote(request.method)) request.decode(strict=False) - args.append(shlex.quote(request.url)) + args = ["http", request.method, request.url] for k, v in request.headers.items(multi=True): - args.append(shlex.quote("%s:%s" % (k, v))) + args.append("%s:%s" % (k, v)) + cmd = ' '.join(shlex.quote(arg) for arg in args) if request.content: try: content = strutils.always_str(request.content) @@ -57,8 +56,8 @@ def httpie_command(f: flow.Flow) -> str: # shlex.quote doesn't support a bytes object # see https://github.com/python/cpython/pull/10871 raise exceptions.CommandError("Request content must be valid unicode") - args += ["<<<", shlex.quote(content)] - return ' '.join(args) + cmd += " <<< " + shlex.quote(content) + return cmd def raw(f: flow.Flow) -> bytes: -- cgit v1.2.3 From d852f292c9a45de7f45cc8537f2aef217259017e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Lang?= Date: Mon, 14 Jan 2019 02:02:07 -0300 Subject: Fix flake8 warning in curl_command There was an unused variable (I redefined its value below). It didn't affect the behavior of the function --- mitmproxy/addons/export.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 04c0349a..a7152ae4 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -34,10 +34,7 @@ def curl_command(f: flow.Flow) -> str: # shlex.quote doesn't support a bytes object # see https://github.com/python/cpython/pull/10871 raise exceptions.CommandError("Request content must be valid unicode") - args += [ - "--data-binary", - strutils.always_str(request.content) - ] + args += ["--data-binary", content] return ' '.join(shlex.quote(arg) for arg in args) -- cgit v1.2.3 From 01ddda75e8aaab542d2c11b9bc9218a94a342f10 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 15 Nov 2019 19:02:59 +0100 Subject: improve curl/httpie export --- mitmproxy/addons/export.py | 73 ++++++++++++++++++++---------------- test/mitmproxy/addons/test_export.py | 16 ++++---- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 20dac655..638a1e2e 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -1,47 +1,60 @@ -import typing import shlex +import typing + +import pyperclip -from mitmproxy import ctx +import mitmproxy.types from mitmproxy import command -from mitmproxy import flow +from mitmproxy import ctx, http from mitmproxy import exceptions -from mitmproxy.utils import strutils +from mitmproxy import flow from mitmproxy.net.http.http1 import assemble -import mitmproxy.types - -import pyperclip +from mitmproxy.utils import strutils -def cleanup_request(f: flow.Flow): +def cleanup_request(f: flow.Flow) -> http.HTTPRequest: if not hasattr(f, "request"): raise exceptions.CommandError("Can't export flow with no request.") - request = f.request.copy() # type: ignore + assert isinstance(f, http.HTTPFlow) + request = f.request.copy() request.decode(strict=False) - # a bit of clean-up - if request.method == 'GET' and request.headers.get("content-length", None) == "0": - request.headers.pop('content-length') - request.headers.pop(':authority', None) + # a bit of clean-up - these headers should be automatically set by curl/httpie + request.headers.pop('content-length') + if request.headers.get("host", "") == request.host: + request.headers.pop("host") + if request.headers.get(":authority", "") == request.host: + request.headers.pop(":authority") return request +def request_content_for_console(request: http.HTTPRequest) -> str: + try: + text = request.get_text(strict=True) + except ValueError: + # shlex.quote doesn't support a bytes object + # see https://github.com/python/cpython/pull/10871 + raise exceptions.CommandError("Request content must be valid unicode") + escape_control_chars = {chr(i): f"\\x{i:02x}" for i in range(32)} + return "".join( + escape_control_chars.get(x, x) + for x in text + ) + + def curl_command(f: flow.Flow) -> str: request = cleanup_request(f) args = ["curl"] for k, v in request.headers.items(multi=True): - args += ["--compressed "] if k == 'accept-encoding' else [] - args += ["-H", "%s:%s" % (k, v)] + if k.lower() == "accept-encoding": + args.append("--compressed") + else: + args += ["-H", f"{k}: {v}"] if request.method != "GET": args += ["-X", request.method] args.append(request.url) if request.content: - try: - content = strutils.always_str(request.content) - except UnicodeDecodeError: - # shlex.quote doesn't support a bytes object - # see https://github.com/python/cpython/pull/10871 - raise exceptions.CommandError("Request content must be valid unicode") - args += ["--data-binary", content] + args += ["-d", request_content_for_console(request)] return ' '.join(shlex.quote(arg) for arg in args) @@ -49,16 +62,10 @@ def httpie_command(f: flow.Flow) -> str: request = cleanup_request(f) args = ["http", request.method, request.url] for k, v in request.headers.items(multi=True): - args.append("%s:%s" % (k, v)) + args.append(f"{k}: {v}") cmd = ' '.join(shlex.quote(arg) for arg in args) if request.content: - try: - content = strutils.always_str(request.content) - except UnicodeDecodeError: - # shlex.quote doesn't support a bytes object - # see https://github.com/python/cpython/pull/10871 - raise exceptions.CommandError("Request content must be valid unicode") - cmd += " <<< " + shlex.quote(content) + cmd += " <<< " + shlex.quote(request_content_for_console(request)) return cmd @@ -67,9 +74,9 @@ def raw(f: flow.Flow) -> bytes: formats = dict( - curl = curl_command, - httpie = httpie_command, - raw = raw, + curl=curl_command, + httpie=httpie_command, + raw=raw, ) diff --git a/test/mitmproxy/addons/test_export.py b/test/mitmproxy/addons/test_export.py index a4c10c1a..1c16afb2 100644 --- a/test/mitmproxy/addons/test_export.py +++ b/test/mitmproxy/addons/test_export.py @@ -42,22 +42,23 @@ def tcp_flow(): class TestExportCurlCommand: def test_get(self, get_request): - result = """curl -H header:qvalue 'http://address:22/path?a=foo&a=bar&b=baz'""" + result = """curl -H 'header: qvalue' 'http://address:22/path?a=foo&a=bar&b=baz'""" assert export.curl_command(get_request) == result def test_post(self, post_request): post_request.request.content = b'nobinarysupport' - result = "curl -H content-length:15 -X POST http://address:22/path --data-binary nobinarysupport" + result = "curl -X POST http://address:22/path -d nobinarysupport" assert export.curl_command(post_request) == result def test_fails_with_binary_data(self, post_request): # shlex.quote doesn't support a bytes object # see https://github.com/python/cpython/pull/10871 + post_request.request.headers["Content-Type"] = "application/json; charset=utf-8" with pytest.raises(exceptions.CommandError): export.curl_command(post_request) def test_patch(self, patch_request): - result = """curl -H header:qvalue -H content-length:7 -X PATCH 'http://address:22/path?query=param' --data-binary content""" + result = """curl -H 'header: qvalue' -X PATCH 'http://address:22/path?query=param' -d content""" assert export.curl_command(patch_request) == result def test_tcp(self, tcp_flow): @@ -73,28 +74,29 @@ class TestExportCurlCommand: ) ) command = export.curl_command(request) - assert shlex.split(command)[-2] == '--data-binary' + assert shlex.split(command)[-2] == '-d' assert shlex.split(command)[-1] == "'&#" class TestExportHttpieCommand: def test_get(self, get_request): - result = """http GET 'http://address:22/path?a=foo&a=bar&b=baz' header:qvalue""" + result = """http GET 'http://address:22/path?a=foo&a=bar&b=baz' 'header: qvalue'""" assert export.httpie_command(get_request) == result def test_post(self, post_request): post_request.request.content = b'nobinarysupport' - result = "http POST http://address:22/path content-length:15 <<< nobinarysupport" + result = "http POST http://address:22/path <<< nobinarysupport" assert export.httpie_command(post_request) == result def test_fails_with_binary_data(self, post_request): # shlex.quote doesn't support a bytes object # see https://github.com/python/cpython/pull/10871 + post_request.request.headers["Content-Type"] = "application/json; charset=utf-8" with pytest.raises(exceptions.CommandError): export.httpie_command(post_request) def test_patch(self, patch_request): - result = """http PATCH 'http://address:22/path?query=param' header:qvalue content-length:7 <<< content""" + result = """http PATCH 'http://address:22/path?query=param' 'header: qvalue' <<< content""" assert export.httpie_command(patch_request) == result def test_tcp(self, tcp_flow): -- cgit v1.2.3 From 484e099eb10897b89d83d4b0441b4455faab9422 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 15 Nov 2019 20:57:03 +0100 Subject: test coverage++ --- mitmproxy/addons/export.py | 1 + test/mitmproxy/addons/test_export.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 638a1e2e..80413ac9 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -30,6 +30,7 @@ def cleanup_request(f: flow.Flow) -> http.HTTPRequest: def request_content_for_console(request: http.HTTPRequest) -> str: try: text = request.get_text(strict=True) + assert text except ValueError: # shlex.quote doesn't support a bytes object # see https://github.com/python/cpython/pull/10871 diff --git a/test/mitmproxy/addons/test_export.py b/test/mitmproxy/addons/test_export.py index 1c16afb2..4b905722 100644 --- a/test/mitmproxy/addons/test_export.py +++ b/test/mitmproxy/addons/test_export.py @@ -77,6 +77,14 @@ class TestExportCurlCommand: assert shlex.split(command)[-2] == '-d' assert shlex.split(command)[-1] == "'&#" + def test_strip_unnecessary(self, get_request): + get_request.request.headers.clear() + get_request.request.headers["host"] = "address" + get_request.request.headers[":authority"] = "address" + get_request.request.headers["accept-encoding"] = "br" + result = """curl --compressed 'http://address:22/path?a=foo&a=bar&b=baz'""" + assert export.curl_command(get_request) == result + class TestExportHttpieCommand: def test_get(self, get_request): -- cgit v1.2.3