Skip to content

Commit 036c389

Browse files
authored
chore(asm): iast fix vulnerability report hash (#5530)
IAST: Change the vulnerability hash field to use a simpler algorithm that will return the same result among different executions. ## 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/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] PR description includes explicit acknowledgement/acceptance of the performance implications of this PR as reported in the benchmarks PR comment. ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
1 parent 0658ed5 commit 036c389

File tree

2 files changed

+19
-8
lines changed

2 files changed

+19
-8
lines changed

ddtrace/appsec/iast/reporter.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
from typing import Set
2+
import zlib
23

34
import attr
45

6+
from ddtrace.internal.compat import PY2
7+
58

69
@attr.s(eq=True, hash=True)
710
class Evidence(object):
@@ -13,22 +16,20 @@ class Evidence(object):
1316
class Location(object):
1417
path = attr.ib(type=str)
1518
line = attr.ib(type=int)
16-
spanId = attr.ib(type=int, eq=False, hash=False)
19+
spanId = attr.ib(type=int, eq=False, hash=False, repr=False)
1720

1821

1922
@attr.s(eq=True, hash=True)
2023
class Vulnerability(object):
2124
type = attr.ib(type=str)
22-
evidence = attr.ib(type=Evidence)
25+
evidence = attr.ib(type=Evidence, repr=False)
2326
location = attr.ib(type=Location)
24-
hash = attr.ib(init=False, eq=False, hash=False)
27+
hash = attr.ib(init=False, eq=False, hash=False, repr=False)
2528

2629
def __attrs_post_init__(self):
27-
if self.evidence.value is not None:
28-
self.hash = hash(self.type) ^ hash(self.evidence) ^ hash(self.location)
29-
else:
30-
valueparts = (vp["value"] for vp in self.evidence.valueParts)
31-
self.hash = hash(self.type) ^ hash(valueparts) ^ hash(self.location)
30+
self.hash = zlib.crc32(repr(self).encode())
31+
if PY2 and self.hash < 0:
32+
self.hash += 1 << 32
3233

3334

3435
@attr.s(eq=True, hash=True)

tests/appsec/iast/test_weak_hash.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def test_weak_hash_hashlib(iast_span_defaults, hash_func, method):
2525
assert list(span_report.vulnerabilities)[0].location.path == WEAK_ALGOS_FIXTURES_PATH
2626
assert list(span_report.vulnerabilities)[0].location.line == 14 if sys.version_info > (3, 0, 0) else 11
2727
assert list(span_report.vulnerabilities)[0].evidence.value == hash_func
28+
assert list(span_report.vulnerabilities)[0].hash == 2491892610 if sys.version_info > (3, 0, 0) else 2454487401
2829

2930

3031
@pytest.mark.skipif(sys.version_info < (3, 0, 0), reason="Digest is wrapped in Python 3")
@@ -60,6 +61,7 @@ def test_weak_hash_new(iast_span_defaults):
6061
assert list(span_report.vulnerabilities)[0].location.path == WEAK_ALGOS_FIXTURES_PATH
6162
assert list(span_report.vulnerabilities)[0].location.line == 23 if sys.version_info > (3, 0, 0) else 20
6263
assert list(span_report.vulnerabilities)[0].evidence.value == "md5"
64+
assert list(span_report.vulnerabilities)[0].hash == 2206071529 if sys.version_info > (3, 0, 0) else 2168145072
6365

6466

6567
def test_weak_hash_new_with_child_span(tracer, iast_span_defaults):
@@ -72,10 +74,12 @@ def test_weak_hash_new_with_child_span(tracer, iast_span_defaults):
7274
assert list(span_report1.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
7375
assert list(span_report1.vulnerabilities)[0].location.path == WEAK_ALGOS_FIXTURES_PATH
7476
assert list(span_report1.vulnerabilities)[0].evidence.value == "md5"
77+
assert list(span_report1.vulnerabilities)[0].hash == 2206071529 if sys.version_info > (3, 0, 0) else 2168145072
7578

7679
assert list(span_report2.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
7780
assert list(span_report2.vulnerabilities)[0].location.path == WEAK_ALGOS_FIXTURES_PATH
7881
assert list(span_report2.vulnerabilities)[0].evidence.value == "md5"
82+
assert list(span_report2.vulnerabilities)[0].hash == 2206071529 if sys.version_info > (3, 0, 0) else 2168145072
7983

8084

8185
@pytest.mark.skipif(sys.version_info < (3, 0, 0), reason="_md5 works only in Python 3")
@@ -105,6 +109,7 @@ def test_weak_hash_md5_builtin_py3_md5_and_sha1_configured(iast_span_defaults):
105109
assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
106110
assert list(span_report.vulnerabilities)[0].location.path == WEAK_HASH_FIXTURES_PATH
107111
assert list(span_report.vulnerabilities)[0].evidence.value == "md5"
112+
assert list(span_report.vulnerabilities)[0].hash == 2972079025
108113

109114

110115
@pytest.mark.skipif(sys.version_info < (3, 0, 0), reason="_md5 works only in Python 3")
@@ -133,6 +138,7 @@ def test_weak_hash_md5_builtin_py3_only_md5_configured(iast_span_only_md5):
133138
assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
134139
assert list(span_report.vulnerabilities)[0].location.path == WEAK_HASH_FIXTURES_PATH
135140
assert list(span_report.vulnerabilities)[0].evidence.value == "md5"
141+
assert list(span_report.vulnerabilities)[0].hash == 2715107846
136142

137143

138144
@pytest.mark.skipif(sys.version_info < (3, 0, 0), reason="_md5 works only in Python 3")
@@ -173,6 +179,7 @@ def test_weak_hash_pycryptodome_hashes_md5(iast_span_defaults):
173179
assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
174180
assert list(span_report.vulnerabilities)[0].location.path == WEAK_HASH_FIXTURES_PATH
175181
assert list(span_report.vulnerabilities)[0].evidence.value == "md5"
182+
assert list(span_report.vulnerabilities)[0].hash == 758317375
176183

177184

178185
def test_weak_hash_pycryptodome_hashes_sha1_defaults(iast_span_defaults):
@@ -187,6 +194,7 @@ def test_weak_hash_pycryptodome_hashes_sha1_defaults(iast_span_defaults):
187194
assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
188195
assert list(span_report.vulnerabilities)[0].location.path == WEAK_HASH_FIXTURES_PATH
189196
assert list(span_report.vulnerabilities)[0].evidence.value == "sha1"
197+
assert list(span_report.vulnerabilities)[0].hash == 3378580158
190198

191199

192200
def test_weak_hash_pycryptodome_hashes_sha1_only_md5_configured(iast_span_only_md5):
@@ -212,7 +220,9 @@ def test_weak_hash_pycryptodome_hashes_sha1_only_sha1_configured(iast_span_only_
212220

213221
assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_HASHING_TYPE
214222
assert list(span_report.vulnerabilities)[0].location.path == WEAK_HASH_FIXTURES_PATH
223+
assert list(span_report.vulnerabilities)[0].location.line == 218
215224
assert list(span_report.vulnerabilities)[0].evidence.value == "sha1"
225+
assert list(span_report.vulnerabilities)[0].hash == 1151623950
216226

217227

218228
def test_weak_check_repeated(iast_span_defaults):

0 commit comments

Comments
 (0)