Fixes #19 (command injection in data2req templating)

[Samdaaman]

Notes

• Added validate_request_method to make sure the the request method is "get", "post", "patch" etc so that the call requests.{request_method} is actually a valid function
• Added escape_request_path to escape the following chars in the request path ', ", {, }, \.
  • Escaping quotes and backslashes is needed so the request_path can't be injected by terminating the string early
  • Escaping curly brackets is needed as we are using a format string and don't want to inject any parameters https://stackoverflow.com/a/5466478
• Replaced .format(host) with an f-string (been around since python 3.6 and is more readable IMO)

Fixes #19

Test cases

Example 1 - bad path (command injection)

Any backslash or quote characters in the path are escaped by an additional backslash

Request

req = (
    f'GET /"+__import__("os").system("whoami")+"/path HTTP/1.1\n'
    f'Host: 192.168.1.107:1337\n'
    f'\n'
)

# RAW
"""
GET /"+__import__("os").system("whoami")+"/path HTTP/1.1
Host: 192.168.1.107:1337

"""

Tulip output (PythonRequest View)

...
requests.get(f"http://{host}:1337/\"+__import__(\"os\").system(\"whoami\")+\"/path", data=data, headers=headers)

Tulip output (Copy as Requests)

...
s.get(f"http://{host}:1337/\"+__import__(\"os\").system(\"whoami\")+\"/path", data=data)

Example 2 - bad path

Just tried spamming bad characters to make sure the escaping worked. Note the curly braces in the path get doubled.

Request

req = (
    f'''GET /+\\"'\\\\""'\\'"'\\'"\\"{{}}{{1}}{{-1}}{{name}}''\\\\\\'"'\\\\\\"\\' HTTP/1.1\n'''
    f'Host: 192.168.1.107:1337\n'
    f'\n'
)

# RAW
"""
GET /+\"'\\""'\'"'\'"\"{}{1}{-1}{name}''\\\'"'\\\"\' HTTP/1.1
Host: 192.168.1.107:1337

"""

Tulip output (PythonRequest View)

...
requests.get(f"http://{host}:1337/+\\\"\'\\\\\"\"\'\\\'\"\'\\\'\"\\\"{{}}{{1}}{{-1}}{{name}}\'\'\\\\\\\'\"\'\\\\\\\"\\\'", data=data, headers=headers)

Tulip output (Copy as Requests)

...
s.get(f"http://{host}:1337/+\\\"\'\\\\\"\"\'\\\'\"\'\\\'\"\\\"{{}}{{1}}{{-1}}{{name}}\'\'\\\\\\\'\"\'\\\\\\\"\\\'", data=data)

Example 3 - bad request method

Here there was no point generating a python3 requests script as the request method used didn't conform to the usual "GET"/"POST"/"PATCH" etc. I opted to raise an Exception here, but could potentially just call requests.unknown instead if throwing was undesirable?

Request

req = (
    f'get(__import__("os").system("whoami"))# /path HTTP/1.1\n'
    f'Host: 192.168.1.107:1337\n'
    f'\n'
)

# RAW
"""
get(__import__("os").system("whoami"))# /path HTTP/1.1
Host: 192.168.1.107:1337

"""

Tulip output (both PythonRequest View and Copy as Requests)

There was an error while converting the request:
Exception: Invalid request method: get(__import__("os").system("whoami"))#

[ItsShadowCone]

Thanks for the PR! However, after sleeping on it, I think I'd prefer the simple repr() solution over custom escaping and format strings. Maybe just make it emit f"http://{host}:port" + "/whatever" by chaining the constant string f"... " + with a repr(parameter) while only ensuring that / needs to be present alongside the given http method (thanks for that check tho!)

After quick googling I couldn't find an api documentation contract that specifies, that repr(str) can be safely eval-ed to produce the given str, but I think simplicity is the better solution here.

[Samdaaman]

Hey mate - I think you are right about the repr. From the docs

For many types, this function makes an attempt to return a string that would yield an object with the same value when passed to eval() Source

Updated the PR. For Examples 1 & 2 above, the output has changed to this format now:

...
s.get(f"http://{host}:1337" + '/+\\"\'\\\\""\'\\\'"\'\\\'"\\"{}{1}{-1}{name}\'\'\\\\\\\'"\'\\\\\\"\\\'', data=data)

...
s.get(f"http://{host}:1337" + '/"+__import__("os").system("whoami")+"/path', data=data)

Added an additional check to make sure request.path.startwith('/') as requested

[Sijisu]

LGTM. If @DaClemens is happy as well, we can merge this

[ItsShadowCone]

Happy, but haven't tested it.

[RickdeJager]

Thanks, merged!