Skip to content

Commit 3e058eb

Browse files
authored
fix(asm): avoid json decode error in request body (backport #4129) (#4130)
This is an automatic backport of pull request #4129 done by [Mergify](https://mergify.com). Cherry-pick of b8ddbec has failed: ``` On branch mergify/bp/1.4/pr-4129 Your branch is up to date with 'origin/1.4'. You are currently cherry-picking commit b8ddbec. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: new file: releasenotes/notes/asm-avoid-json-decode-error-31479891110bcb55.yaml modified: tests/contrib/django/test_django_appsec.py Unmerged paths: (use "git add <file>..." to mark resolution) both modified: ddtrace/contrib/django/utils.py both modified: ddtrace/contrib/flask/patch.py both modified: ddtrace/contrib/pylons/middleware.py both modified: tests/contrib/flask/test_flask_appsec.py both modified: tests/contrib/pylons/test_pylons.py ``` To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details>
1 parent d0268d0 commit 3e058eb

File tree

5 files changed

+67
-14
lines changed

5 files changed

+67
-14
lines changed

ddtrace/contrib/django/utils.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@
2525
from .compat import user_is_authenticated
2626

2727

28+
try:
29+
from json import JSONDecodeError
30+
except ImportError:
31+
# handling python 2.X import error
32+
JSONDecodeError = ValueError # type: ignore
33+
34+
2835
log = get_logger(__name__)
2936

3037
# Set on patch, when django is imported
@@ -312,7 +319,7 @@ def _after_request_tags(pin, span, request, response):
312319

313320
if config._appsec_enabled and request.method in _BODY_METHODS:
314321
content_type = (
315-
request.content_type if hasattr(request, "content_type") else request.META["CONTENT_TYPE"]
322+
request.content_type if hasattr(request, "content_type") else request.META.get("CONTENT_TYPE")
316323
)
317324

318325
rest_framework = hasattr(request, "data")
@@ -328,7 +335,14 @@ def _after_request_tags(pin, span, request, response):
328335
)
329336
else: # text/plain, xml, others: take them as strings
330337
req_body = request.data.decode("UTF-8") if rest_framework else request.body.decode("UTF-8")
331-
except (AttributeError, RawPostDataException, UnreadablePostError, OSError):
338+
except (
339+
AttributeError,
340+
RawPostDataException,
341+
UnreadablePostError,
342+
OSError,
343+
ValueError,
344+
JSONDecodeError,
345+
):
332346
log.warning("Failed to parse request body", exc_info=True)
333347
# req_body is None
334348

ddtrace/contrib/flask/patch.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@
3434
from .wrappers import wrap_signal
3535

3636

37+
try:
38+
from json import JSONDecodeError
39+
except ImportError:
40+
# handling python 2.X import error
41+
JSONDecodeError = ValueError # type: ignore
42+
43+
3744
log = get_logger(__name__)
3845

3946
FLASK_ENDPOINT = "flask.endpoint"
@@ -374,7 +381,7 @@ def traced_wsgi_app(pin, wrapped, instance, args, kwargs):
374381
req_body = request.form.to_dict()
375382
else:
376383
req_body = request.get_data()
377-
except (AttributeError, RuntimeError, TypeError, BadRequest):
384+
except (AttributeError, RuntimeError, TypeError, BadRequest, ValueError, JSONDecodeError):
378385
log.warning("Failed to parse werkzeug request body", exc_info=True)
379386
finally:
380387
# Reset wsgi input to the beginning
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
ASM: (flask) avoid json decode error while parsing request body.

tests/contrib/django/test_django_appsec.py

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import logging
23

34
from ddtrace.internal import _context
45
from ddtrace.internal.compat import urlencode
@@ -157,18 +158,31 @@ def test_django_request_body_plain(client, test_spans, tracer):
157158

158159

159160
def test_django_request_body_plain_attack(client, test_spans, tracer):
160-
with override_global_config(dict(_appsec_enabled=True)):
161-
with override_env(dict(DD_APPSEC_RULES=RULES_GOOD_PATH)):
162-
tracer._appsec_enabled = True
163-
# Hack: need to pass an argument to configure so that the processors are recreated
164-
tracer.configure(api_version="v0.4")
165-
payload = "1' or '1' = '1'"
166-
client.post("/", payload, content_type="text/plain")
167-
root_span = test_spans.spans[0]
161+
with override_global_config(dict(_appsec_enabled=True)), override_env(dict(DD_APPSEC_RULES=RULES_GOOD_PATH)):
162+
tracer._appsec_enabled = True
163+
# Hack: need to pass an argument to configure so that the processors are recreated
164+
tracer.configure(api_version="v0.4")
165+
payload = "1' or '1' = '1'"
166+
client.post("/", payload, content_type="text/plain")
167+
root_span = test_spans.spans[0]
168168

169-
query = _context.get_item("http.request.body", span=root_span)
170-
assert "triggers" in json.loads(root_span.get_tag("_dd.appsec.json"))
171-
assert query == "1' or '1' = '1'"
169+
query = _context.get_item("http.request.body", span=root_span)
170+
assert "triggers" in json.loads(root_span.get_tag("_dd.appsec.json"))
171+
assert query == "1' or '1' = '1'"
172+
173+
174+
# @pytest.fixture(autouse=True)
175+
def test_django_request_body_json_empty(caplog, client, test_spans, tracer):
176+
with caplog.at_level(logging.WARNING), override_global_config(dict(_appsec_enabled=True)), override_env(
177+
dict(DD_APPSEC_RULES=RULES_GOOD_PATH)
178+
):
179+
tracer._appsec_enabled = True
180+
# Hack: need to pass an argument to configure so that the processors are recreated
181+
tracer.configure(api_version="v0.4")
182+
payload = '{"attack": "bad_payload",}'
183+
response = client.post("/", payload, content_type="application/json")
184+
assert response.status_code == 200
185+
assert "Failed to parse request body" in caplog.text
172186

173187

174188
def test_django_path_params(client, test_spans, tracer):

tests/contrib/flask/test_flask_appsec.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import json
2+
import logging
23

34
from flask import request
5+
import pytest
46

57
from ddtrace.internal import _context
68
from ddtrace.internal.compat import urlencode
@@ -11,6 +13,10 @@
1113

1214

1315
class FlaskAppSecTestCase(BaseFlaskTestCase):
16+
@pytest.fixture(autouse=True)
17+
def inject_fixtures(self, caplog):
18+
self._caplog = caplog
19+
1420
def test_flask_simple_attack(self):
1521
self.tracer._appsec_enabled = True
1622
# Hack: need to pass an argument to configure so that the processors are recreated
@@ -188,3 +194,11 @@ def test_flask_body_json_attack(self):
188194
query = dict(_context.get_item("http.request.body", span=root_span))
189195
assert "triggers" in json.loads(root_span.get_tag("_dd.appsec.json"))
190196
assert query == {"attack": "1' or '1' = '1'"}
197+
198+
def test_flask_body_json_empty_body_logs_warning(self):
199+
with self._caplog.at_level(logging.WARNING), override_global_config(dict(_appsec_enabled=True)):
200+
self.tracer._appsec_enabled = True
201+
# Hack: need to pass an argument to configure so that the processors are recreated
202+
self.tracer.configure(api_version="v0.4")
203+
self.client.post("/", data="", content_type="application/json")
204+
assert "Failed to parse werkzeug request body" in self._caplog.text

0 commit comments

Comments
 (0)