Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions app/access_grant_funding/routes/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,55 @@ def export_submission_pdf(
)


@access_grant_funding_blueprint.route(
"/organisation/<uuid:organisation_id>/grants/<uuid:grant_id>/<collection_type:collection_type>/<uuid:submission_id>/all-questions",
methods=["GET"],
)
@has_access_grant_role(RoleEnum.MEMBER)
def all_questions(
organisation_id: UUID, grant_id: UUID, collection_type: CollectionType, submission_id: UUID
) -> ResponseReturnValue:
grant_recipient = get_grant_recipient(grant_id, organisation_id)

submission = SubmissionHelper.load(submission_id=submission_id, grant_recipient_id=grant_recipient.id)

return render_template(
"access_grant_funding/collections/all_questions.html",
grant_recipient=grant_recipient,
submission=submission,
interpolate=SubmissionHelper.get_print_interpolator(submission.collection),
)


@access_grant_funding_blueprint.route(
"/organisation/<uuid:organisation_id>/grants/<uuid:grant_id>/<collection_type:collection_type>/<uuid:submission_id>/all-questions/pdf",
methods=["GET"],
)
@has_access_grant_role(RoleEnum.MEMBER)
def all_questions_pdf(
organisation_id: UUID, grant_id: UUID, collection_type: CollectionType, submission_id: UUID
) -> ResponseReturnValue:
grant_recipient = get_grant_recipient(grant_id, organisation_id)

helper = SubmissionHelper.load(submission_id=submission_id, grant_recipient_id=grant_recipient.id)

html_content = render_template(
"common/all_questions_print_baseline.html",
collection=helper.collection,
interpolate=SubmissionHelper.get_print_interpolator(helper.collection),
)

emit_metric_count(MetricEventName.ACCESS_ALL_QUESTIONS_PDF_DOWNLOADED, submission=helper.submission)

return send_file(
io.BytesIO(render_pdf(html_content)),
mimetype="application/pdf",
as_attachment=True,
download_name=secure_filename(f"{helper.collection.grant.name} - {helper.collection.name} - all questions.pdf"),
max_age=0,
)
Comment on lines +255 to +261

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium鈥係AST Finding

Potential Path Traversal Vulnerability in Flask (CWE-22)

More Details

This rule detects a potential path traversal vulnerability in Flask applications. Path traversal vulnerabilities occur when user-controlled data is used directly in file system operations without proper sanitization. This could allow an attacker to access or manipulate arbitrary files on the server's file system, potentially leading to data breaches, code execution, or system compromise.

The vulnerability arises when user input from Flask request parameters (such as request.args, request.form, request.values, request.json, or request.data) is used directly in file operations like open() or pathlib.Path() without validating or sanitizing the input. An attacker could craft malicious input containing directory traversal sequences (e.g., '../') to access files outside the intended directory.

To mitigate this risk, user input should never be trusted and must be properly validated and sanitized before using it in file system operations. Implement input validation checks, remove or replace directory traversal sequences, and restrict file access to a whitelisted directory.

Attribute Value
Impact Medium
Likelihood Medium

Remediation

Path traversal vulnerabilities allow an attacker to access or manipulate files outside of the intended directory on the server's file system. This can lead to unauthorized data access, data tampering, or even remote code execution, posing a severe security risk.

To mitigate this vulnerability, user-supplied input should never be directly used in file operations without proper sanitization. Instead, implement strict input validation and path canonicalization to prevent path traversal attacks. Sanitize user input by removing or encoding special characters like "../" that could be used for directory traversal. Additionally, use secure APIs that provide path normalization and restrict access to sensitive directories.

Code examples

# VULNERABLE CODE - User input is directly used in file operations without sanitization
file_path = request.args.get('file')
with open(file_path, 'r') as f:
    contents = f.read()
# SECURE CODE - User input is sanitized, and path is normalized using secure APIs
import os
from pathlib import Path

file_name = request.args.get('file')
# Sanitize user input
file_name = os.path.basename(file_name)
# Normalize path and restrict access to the 'files' directory
file_path = Path('files', file_name).resolve()
if not file_path.is_file() or not file_path.parent == Path('files'):
    raise ValueError('Invalid file path')
with file_path.open('r') as f:
    contents = f.read()

Additional recommendations

  • Follow the principle of least privilege and restrict file access permissions as much as possible.
  • Implement strict input validation using allowlists (whitelists) instead of denylists (blacklists).
  • Consider using web application firewalls (WAFs) or content security policies (CSPs) as additional layers of defense.
  • Adhere to the OWASP Top 10 Web Application Security Risks and the OWASP Cheat Sheet Series for secure coding practices.
  • Regularly update your dependencies and frameworks to ensure you have the latest security patches.

Rule ID: WS-PYTHON-00326


To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason

If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).


To get more details on how to remediate this issue using AI, reply to this conversation with #wiz remediate



@access_grant_funding_blueprint.route(
"/organisation/<uuid:organisation_id>/grants/<uuid:grant_id>/<collection_type:collection_type>/<uuid:submission_id>/decline",
methods=["GET", "POST"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %}
{% from "common/macros/all_questions.html" import render_all_questions with context %}
{% extends "access_grant_funding/base.html" %}

{% set collection = submission.collection %}
{% set page_title = "All questions - " ~ submission.long_collection_name %}
{% set active_item_identifier = collection.type.constants.active_nav %}

{% block beforeContent %}
{{ super() }}
{{
govukBackLink({
"href": url_for("access_grant_funding.tasklist", organisation_id=grant_recipient.organisation.id, grant_id=grant_recipient.grant.id, collection_type=collection.type, submission_id=submission.id),
"text": "Back"
})
}}
{% endblock beforeContent %}

{% block content %}
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<span class="govuk-caption-l">{{ collection.name }}</span>
<h1 class="govuk-heading-l">All questions</h1>

<a
href="{{ url_for('access_grant_funding.all_questions_pdf', organisation_id=grant_recipient.organisation.id, grant_id=grant_recipient.grant.id, collection_type=collection.type, submission_id=submission.id) }}"
class="govuk-button govuk-button--secondary"
download>
Download as PDF
</a>

{{ render_all_questions(collection) }}
</div>
</div>
{% endblock content %}
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,15 @@ <h2 class="govuk-heading-m">Submit your {{ constants.singular }}</h2>
{% set submit_text="Continue to submit" %}
{% endif %}

{{ collection_tasklist(runner, submit_text, before_submission=before_submission) }}
{%
set all_questions_href = url_for(
"access_grant_funding.all_questions",
organisation_id=grant_recipient.organisation.id,
grant_id=grant_recipient.grant.id,
collection_type=runner.submission.collection.type,
submission_id=runner.submission.id
)
%}

{{ collection_tasklist(runner, submit_text, before_submission=before_submission, all_questions_href=all_questions_href) }}
{% endblock content %}
7 changes: 6 additions & 1 deletion app/common/templates/common/macros/collections.html
Original file line number Diff line number Diff line change
Expand Up @@ -390,11 +390,12 @@ <h1 class="govuk-heading-l">{{ "Your submitted answers" if (submission.in_answer
{% endif %}
{% endmacro %}

{% macro collection_tasklist(runner, submit_text=None, before_submission=None) %}
{% macro collection_tasklist(runner, submit_text=None, before_submission=None, all_questions_href=None) %}
{% set submission = runner.submission %}

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<h2 class="govuk-heading-m">Sections</h2>
{% set forms = submission.get_ordered_visible_forms() %}
{% if not forms %}
<p class="govuk-body">This collection has no forms.</p>
Expand Down Expand Up @@ -445,6 +446,10 @@ <h1 class="govuk-heading-l">{{ "Your submitted answers" if (submission.in_answer
})
}}
{% endif %}

{% if all_questions_href %}
<p class="govuk-body govuk-!-margin-bottom-6">You can <a class="govuk-link govuk-link--no-visited-state" href="{{ all_questions_href }}">view all questions</a>.</p>
{% endif %}
</div>
</div>
{% set default_before_submission %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
<div class="govuk-grid-column-full">
{{ layout.messages() }}

<span class="govuk-caption-l">View all questions</span>
<h1 class="govuk-heading-l">{{ collection.name }}</h1>
<span class="govuk-caption-l">{{ collection.name }}</span>
<h1 class="govuk-heading-l">All questions</h1>

<a href="{{ url_for('collection.all_questions_pdf', collection_id=collection.id) }}" class="govuk-button govuk-button--secondary" download>Download as PDF</a>

Expand Down
2 changes: 2 additions & 0 deletions app/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class MetricEventName(StrEnum):
SUBMISSIONS_EXPORTED = "submissions-exported"
SUBMISSION_PDF_DOWNLOADED = "submission-pdf-downloaded"

ACCESS_ALL_QUESTIONS_PDF_DOWNLOADED = "access-all-questions-pdf-downloaded"

VALIDATION_CREATED_CUSTOM = "validation-created-custom"
VALIDATION_CREATED_MANAGED = "validation-created-managed"

Expand Down
82 changes: 82 additions & 0 deletions tests/integration/access_grant_funding/routes/test_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,88 @@ def test_redirects_to_route_to_submission_when_data_no_longer_missing_and_multip
)


class TestAllQuestions:
@pytest.mark.parametrize(
"client_fixture, can_access",
(
("authenticated_no_role_client", False),
("authenticated_grant_recipient_member_client", True),
("authenticated_grant_recipient_data_provider_client", True),
),
)
def test_get_all_questions(self, request: FixtureRequest, client_fixture: str, can_access: bool, factories) -> None:
client = request.getfixturevalue(client_fixture)
grant_recipient = getattr(client, "grant_recipient", None) or factories.grant_recipient.create()
question = factories.question.create(
form__title="Colour information",
form__collection__grant=grant_recipient.grant,
text="What is your favourite colour?",
)
collection = question.form.collection
submission = factories.submission.create(
collection=collection, grant_recipient=grant_recipient, mode=SubmissionModeEnum.LIVE
)

response = client.get(
url_for(
"access_grant_funding.all_questions",
organisation_id=grant_recipient.organisation.id,
grant_id=grant_recipient.grant.id,
collection_type=collection.type,
submission_id=submission.id,
)
)

if not can_access:
assert response.status_code == 403
return

assert response.status_code == 200
soup = BeautifulSoup(response.data, "html.parser")
assert get_h1_text(soup) == "All questions"
assert collection.name in soup.text
assert "What is your favourite colour?" in soup.text
assert page_has_link(soup, "Download as PDF")["href"] == url_for(
"access_grant_funding.all_questions_pdf",
organisation_id=grant_recipient.organisation.id,
grant_id=grant_recipient.grant.id,
collection_type=collection.type,
submission_id=submission.id,
)

@patch("app.access_grant_funding.routes.collections.emit_metric_count")
def test_all_questions_pdf(self, mock_count, authenticated_grant_recipient_member_client, factories, mocker):
grant_recipient = authenticated_grant_recipient_member_client.grant_recipient
question = factories.question.create(
form__collection__grant=grant_recipient.grant, text="What is your favourite colour?"
)
submission = factories.submission.create(
collection=question.form.collection, grant_recipient=grant_recipient, mode=SubmissionModeEnum.LIVE
)
render_pdf = mocker.patch(
"app.access_grant_funding.routes.collections.render_pdf", return_value=b"%PDF-1.4 fake"
)

response = authenticated_grant_recipient_member_client.get(
url_for(
"access_grant_funding.all_questions_pdf",
organisation_id=grant_recipient.organisation.id,
grant_id=grant_recipient.grant.id,
collection_type=submission.collection.type,
submission_id=submission.id,
)
)

mock_count.assert_called_once_with(MetricEventName.ACCESS_ALL_QUESTIONS_PDF_DOWNLOADED, submission=submission)

assert response.status_code == 200
assert response.mimetype == "application/pdf"
assert "all_questions" in response.headers["Content-Disposition"]
printed_html = render_pdf.call_args.args[0]
assert "MHCLG Access grant funding" in printed_html
assert "What is your favourite colour?" in printed_html


class TextExportReportPDF:
# the first method under test will spin up chromium which will always be marked as as a slow test
@pytest.mark.fail_slow("1000ms", enabled=False)
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/access_grant_funding/routes/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,8 @@ def test_get_tasklist(
f"/reports/{submission.id}/check-your-answers/{question.form.id}?source=tasklist"
)

assert page_has_link(soup, "view all questions")

def test_get_tasklist_excludes_eligibility_form(
self, authenticated_grant_recipient_data_provider_client, factories
):
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_all_routes_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ def _get_decorators(func):
"access_grant_funding.list_grant_team",
"access_grant_funding.view_locked_submission",
"access_grant_funding.export_submission_pdf",
"access_grant_funding.all_questions",
"access_grant_funding.all_questions_pdf",
"access_grant_funding.submitted_confirmation",
"access_grant_funding.download_file",
"access_grant_funding.collection_unavailable",
Expand Down
Loading