Skip to content

Commit

Permalink
Docs and tests suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
andrecsilva committed Jan 17, 2024
1 parent df231bb commit 46cf352
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
The `send_file` function from Flask is susceptible to a path injection attack if its input is not proper validated.
In a path injection attack, the malicious agent can craft a path containing special paths like `./` or `../` to resolve an unintended file. This allows the agent to overwrite, delete or read arbitrary files.
The `Flask` `send_file` function from Flask is susceptible to a path traversal attack if its input is not properly validated.
In a path traversal attack, the malicious agent can craft a path containing special paths like `./` or `../` to resolve a file outside of the expected directory path. This potentially allows the agent to overwrite, delete or read arbitrary files. In the case of `flask.send_file`, the result is that a malicious user could potentially download sensitive files that exist on the filesystem where the application is being hosted.
Flask offers a native solution with the `flask.send_from_directory` function that validates the given path.

Our changes look something like this:
Expand Down
2 changes: 1 addition & 1 deletion src/core_codemods/flask_send_file_path_parameterization.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class FlaskSendFilePathParameterization(BaseCodemod, NameAndAncestorResolutionMixin):
NAME = "flask-send-file-path-parameterization"
SUMMARY = "Replaces `send_file` with `send_from_directory`"
SUMMARY = "Replace unsafe usage of `flask.send_file`"
DESCRIPTION = SUMMARY
REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW
REFERENCES = [
Expand Down
39 changes: 31 additions & 8 deletions tests/codemods/test_flask_send_file_path_parameterization.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
FlaskSendFilePathParameterization,
)
from tests.codemods.base_codemod_test import BaseCodemodTest
from textwrap import dedent


class TestFlaskSendFilePathParameterization(BaseCodemodTest):
Expand Down Expand Up @@ -32,7 +31,31 @@ def download_file(name):
def download_file(name):
return flask.send_from_directory((p := Path(f'path/to/{name}.txt')).parent, p.name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_direct_simple_string(self, tmpdir):
input_code = """\
from flask import Flask, send_file
app = Flask(__name__)
@app.route("/uploads/<path:name>")
def download_file(name):
return send_file('path/to/file.txt')
"""
expected = """\
from flask import Flask
import flask
from pathlib import Path
app = Flask(__name__)
@app.route("/uploads/<path:name>")
def download_file(name):
return flask.send_from_directory((p := Path('path/to/file.txt')).parent, p.name)
"""
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_direct_string_convert_arguments(self, tmpdir):
Expand All @@ -56,7 +79,7 @@ def download_file(name):
def download_file(name):
return flask.send_from_directory((p := Path(f'path/to/{name}.txt')).parent, p.name, mimetype = None, as_attachment = False, download_name = True)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_direct_path(self, tmpdir):
Expand All @@ -81,7 +104,7 @@ def download_file(name):
def download_file(name):
return flask.send_from_directory((p := Path(f'path/to/{name}.txt')).parent, p.name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_indirect_path(self, tmpdir):
Expand All @@ -108,7 +131,7 @@ def download_file(name):
path = Path(f'path/to/{name}.txt')
return flask.send_from_directory(path.parent, path.name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_indirect_path_alias(self, tmpdir):
Expand All @@ -135,7 +158,7 @@ def download_file(name):
path = Path(f'path/to/{name}.txt')
return flask.send_from_directory(path.parent, path.name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_indirect_string(self, tmpdir):
Expand All @@ -161,7 +184,7 @@ def download_file(name):
path = f'path/to/{name}.txt'
return flask.send_from_directory((p := Path(path)).parent, p.name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(expected))
self.run_and_assert(tmpdir, input_code, expected)
assert len(self.file_context.codemod_changes) == 1

def test_unknown_type(self, tmpdir):
Expand All @@ -174,5 +197,5 @@ def test_unknown_type(self, tmpdir):
def download_file(name):
return send_file(name)
"""
self.run_and_assert(tmpdir, dedent(input_code), dedent(input_code))
self.run_and_assert(tmpdir, input_code, input_code)
assert len(self.file_context.codemod_changes) == 0

0 comments on commit 46cf352

Please sign in to comment.