Skip to content

Commit

Permalink
👽 [#4908] Add validation for path
Browse files Browse the repository at this point in the history
We check if the path contains '..' to prevent possible path traversal attacks. For example: '..', 'foo/..', '../foo', and 'foo/../bar' are all not allowed
  • Loading branch information
viktorvanwijk committed Jan 21, 2025
1 parent 32fa8ca commit 1deca41
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 4 deletions.
14 changes: 14 additions & 0 deletions src/openforms/registrations/contrib/json_dump/config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from typing import Required, TypedDict

from django.core.exceptions import ValidationError
from django.utils.translation import gettext_lazy as _

from rest_framework import serializers
Expand All @@ -11,6 +12,18 @@
from openforms.utils.mixins import JsonSchemaSerializerMixin


def validate_path(v: str) -> None:
"""Validate path by checking if it contains '..', which can lead to path traversal
attacks.
:param v: path to validate
"""
if ".." in v:
raise ValidationError(
"Path cannot contain '..', as it can lead to path traversal attacks."
)


class JSONDumpOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
service = PrimaryKeyRelatedAsChoicesField(
queryset=Service.objects.filter(api_type=APITypes.orc),
Expand All @@ -29,6 +42,7 @@ class JSONDumpOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serialize
allow_blank=True,
required=False,
default="",
validators=[validate_path],
)
form_variables = serializers.ListField(
child=FormioVariableKeyField(),
Expand Down
10 changes: 6 additions & 4 deletions src/openforms/registrations/contrib/json_dump/plugin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import base64
import json

from django.core.exceptions import SuspiciousOperation
from django.core.serializers.json import DjangoJSONEncoder
from django.utils.translation import gettext_lazy as _

Expand Down Expand Up @@ -62,10 +63,10 @@ def register_submission(
service = options["service"]
submission.registration_result = result = {}
with build_client(service) as client:
res = client.post(
options.get("relative_api_endpoint", ""),
json=data,
)
if ".." in (path := options["relative_api_endpoint"]):
raise SuspiciousOperation("Possible path traversal detected")

res = client.post(path, json=data)
res.raise_for_status()

result["api_response"] = res.json()
Expand Down Expand Up @@ -120,6 +121,7 @@ def process_variables(submission: Submission, values: JSONObject):
f"attachments ({n_attachments}) is not allowed."
)


def encode_attachment(attachment: SubmissionFileAttachment) -> str:
"""Encode an attachment using base64
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from base64 import b64decode
from pathlib import Path

from django.core.exceptions import SuspiciousOperation
from django.test import TestCase

from requests import RequestException
Expand Down Expand Up @@ -178,3 +179,31 @@ def test_multiple_file_uploads(self):
res_json = res["api_response"]

self.assertEqual(res_json["data"]["values"], expected_values)

def test_path_traversal_attack(self):
submission = SubmissionFactory.from_components(
[
{"key": "firstName", "type": "textField"},
{"key": "lastName", "type": "textfield"},
],
completed=True,
submitted_data={
"firstName": "We Are",
"lastName": "Checking",
},
bsn="123456789",
)

json_form_options = dict(
service=(ServiceFactory(api_root="http://localhost:80/")),
path="..",
form_variables=["firstName", "file", "auth_bsn"],
)
json_plugin = JSONDumpRegistration("json_registration_plugin")
set_submission_reference(submission)

for path in ("..", "../foo", "foo/..", "foo/../bar"):
with self.subTest(path):
json_form_options["path"] = path
with self.assertRaises(SuspiciousOperation):
json_plugin.register_submission(submission, json_form_options)
26 changes: 26 additions & 0 deletions src/openforms/registrations/contrib/json_dump/tests/test_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from django.test import TestCase

from openforms.appointments.contrib.qmatic.tests.factories import ServiceFactory

from ..config import JSONDumpOptions, JSONDumpOptionsSerializer


class JSONDumpConfigTests(TestCase):
def test_serializer_raises_validation_error_on_path_traversal(self):
service = ServiceFactory.create(api_root="https://example.com/api/v2")

data: JSONDumpOptions = {
"service": service.pk,
"path": "",
"variables": ["now"],
}

# Ensuring that the options are valid in the first place
serializer = JSONDumpOptionsSerializer(data=data)
self.assertTrue(serializer.is_valid())

for path in ("..", "../foo", "foo/..", "foo/../bar"):
with self.subTest(path):
data["path"] = path
serializer = JSONDumpOptionsSerializer(data=data)
self.assertFalse(serializer.is_valid())

0 comments on commit 1deca41

Please sign in to comment.