diff --git a/src/core_codemods/docs/pixee_python_flask-send-file-path-parameterization.md b/src/core_codemods/docs/pixee_python_flask-send-file-path-parameterization.md index 9600de36..2042114b 100644 --- a/src/core_codemods/docs/pixee_python_flask-send-file-path-parameterization.md +++ b/src/core_codemods/docs/pixee_python_flask-send-file-path-parameterization.md @@ -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: diff --git a/src/core_codemods/flask_send_file_path_parameterization.py b/src/core_codemods/flask_send_file_path_parameterization.py index 196fe240..835c9d60 100644 --- a/src/core_codemods/flask_send_file_path_parameterization.py +++ b/src/core_codemods/flask_send_file_path_parameterization.py @@ -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 = [ diff --git a/tests/codemods/test_flask_send_file_path_parameterization.py b/tests/codemods/test_flask_send_file_path_parameterization.py index 75bc045e..c588c06b 100644 --- a/tests/codemods/test_flask_send_file_path_parameterization.py +++ b/tests/codemods/test_flask_send_file_path_parameterization.py @@ -2,7 +2,6 @@ FlaskSendFilePathParameterization, ) from tests.codemods.base_codemod_test import BaseCodemodTest -from textwrap import dedent class TestFlaskSendFilePathParameterization(BaseCodemodTest): @@ -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/") + 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/") + 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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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