Skip to content

Commit 3f93320

Browse files
authored
fix(iast): insecure cookies error [backport #7893 to 2.3] (#7902)
Backport fed5170 from #7893 to 2.3. Cookies vulnerabilities are only reported if response cookies are insecure. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that.
1 parent c8e243a commit 3f93320

File tree

4 files changed

+6
-24
lines changed

4 files changed

+6
-24
lines changed

ddtrace/contrib/trace_utils.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,6 @@ def set_http_meta(
520520
if _is_iast_enabled():
521521
from ddtrace.appsec._iast.taint_sinks.insecure_cookie import asm_check_cookies
522522

523-
if request_cookies:
524-
asm_check_cookies(request_cookies)
525-
526523
if response_cookies:
527524
asm_check_cookies(response_cookies)
528525

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
Vulnerability Management for Code-level (IAST): Cookies vulnerabilities are only reported
5+
if response cookies are insecure.

tests/contrib/flask/test_flask_appsec_iast.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@
66
from ddtrace.appsec._constants import IAST
77
from ddtrace.appsec._iast import oce
88
from ddtrace.appsec._iast._utils import _is_python_version_supported as python_supported_by_iast
9-
from ddtrace.appsec._iast.constants import VULN_INSECURE_COOKIE
10-
from ddtrace.appsec._iast.constants import VULN_NO_HTTPONLY_COOKIE
11-
from ddtrace.appsec._iast.constants import VULN_NO_SAMESITE_COOKIE
129
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
1310
from ddtrace.contrib.sqlite3.patch import patch
1411
from tests.appsec.iast.iast_utils import get_line_and_hash
@@ -438,12 +435,7 @@ def test_sqli():
438435
assert vulnerability["location"]["path"] == TEST_FILE_PATH
439436
assert vulnerability["hash"] == hash_value
440437

441-
assert {
442-
VULN_SQL_INJECTION,
443-
VULN_INSECURE_COOKIE,
444-
VULN_NO_HTTPONLY_COOKIE,
445-
VULN_NO_SAMESITE_COOKIE,
446-
} == vulnerabilities
438+
assert {VULN_SQL_INJECTION} == vulnerabilities
447439

448440
@pytest.mark.skipif(not python_supported_by_iast(), reason="Python version not supported by IAST")
449441
def test_flask_full_sqli_iast_http_request_parameter(self):

tests/tracer/test_trace_utils.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -490,18 +490,6 @@ def test_set_http_meta_insecure_cookies_iast_disabled(span, int_config):
490490
assert not span_report
491491

492492

493-
@pytest.mark.skipif(
494-
sys.version_info < (3, 6, 0) or sys.version_info >= (3, 12),
495-
reason="Python 3.6+ test, IAST not supported with Python 3.12",
496-
)
497-
def test_set_http_meta_insecure_cookies_iast_enabled(span, int_config):
498-
with override_global_config(dict(_iast_enabled=True, _asm_enabled=True)):
499-
cookies = {"foo": "bar"}
500-
trace_utils.set_http_meta(span, int_config.myint, request_cookies=cookies)
501-
span_report = core.get_item(IAST.CONTEXT_KEY, span=span)
502-
assert span_report.vulnerabilities
503-
504-
505493
@mock.patch("ddtrace.contrib.trace_utils._store_headers")
506494
@pytest.mark.parametrize(
507495
"user_agent_key,user_agent_value,expected_keys,expected",

0 commit comments

Comments
 (0)