Skip to content

Commit 881a656

Browse files
Samk1710Samk1710
committed
Add Tests for Advisory History
Signed-off-by: Sampurna Pyne <sampurnapyne1710@gmail.com>
1 parent 769f53c commit 881a656

6 files changed

Lines changed: 1059 additions & 0 deletions

File tree

‎vulnerabilities/tests/test_advisory_history.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import json
2+
import os
3+
4+
from vulnerabilities.templatetags.diff_advisory_history import format_diff_for_ui
5+
from vulnerabilities.tests.util_tests import check_results_against_json
6+
7+
8+
def test_advisory_history_diffing():
9+
"""
10+
Test the diffing logic for historical advisory snapshots.
11+
"""
12+
from vulnerabilities.utils import diff_advisories_v2
13+
14+
base_dir = os.path.join(os.path.dirname(__file__), "test_data", "advisory_history")
15+
16+
# Test GHSA-72hv-8253-57qq (Sample 1)
17+
# See: https://github.com/github/advisory-database/commits/main/advisories/github-reviewed/2026/02/GHSA-72hv-8253-57qq/GHSA-72hv-8253-57qq.json
18+
sample1_dir = os.path.join(base_dir, "GHSA-72hv-8253-57qq")
19+
with open(os.path.join(sample1_dir, "normalised_history_GHSA-72hv-8253-57qq.json"), "r") as f:
20+
sample1_data = json.load(f)
21+
22+
sample1_diffs = [
23+
diff_advisories_v2(sample1_data[i], sample1_data[i + 1])
24+
for i in range(len(sample1_data) - 1)
25+
]
26+
27+
sample1_expected_file = os.path.join(sample1_dir, "expected_diff_GHSA-72hv-8253-57qq.json")
28+
check_results_against_json(sample1_diffs, sample1_expected_file)
29+
30+
# Test GHSA-6rw7-vpxm-498p (Sample 2)
31+
# See: https://github.com/github/advisory-database/commits/main/advisories/github-reviewed/2025/12/GHSA-6rw7-vpxm-498p/GHSA-6rw7-vpxm-498p.json
32+
sample2_dir = os.path.join(base_dir, "GHSA-6rw7-vpxm-498p")
33+
with open(os.path.join(sample2_dir, "normalised_history_GHSA-6rw7-vpxm-498p.json"), "r") as f:
34+
sample2_data = json.load(f)
35+
36+
sample2_diffs = [
37+
diff_advisories_v2(sample2_data[i], sample2_data[i + 1])
38+
for i in range(len(sample2_data) - 1)
39+
]
40+
41+
sample2_expected_file = os.path.join(sample2_dir, "expected_diff_GHSA-6rw7-vpxm-498p.json")
42+
check_results_against_json(sample2_diffs, sample2_expected_file)
43+
44+
45+
def test_format_diff_for_ui():
46+
"""
47+
Test the template tag logic that formats diffs for the UI.
48+
"""
49+
base_dir = os.path.join(os.path.dirname(__file__), "test_data", "advisory_history")
50+
sample2_dir = os.path.join(base_dir, "GHSA-6rw7-vpxm-498p")
51+
input_file = os.path.join(sample2_dir, "expected_diff_GHSA-6rw7-vpxm-498p.json")
52+
53+
with open(input_file, "r") as f:
54+
input_diffs = json.load(f)
55+
56+
formatted_diffs = [format_diff_for_ui(diff) for diff in input_diffs]
57+
58+
expected_file = os.path.join(sample2_dir, "expected_formatted_diff_GHSA-6rw7-vpxm-498p.json")
59+
check_results_against_json(formatted_diffs, expected_file)

‎vulnerabilities/tests/test_data/advisory_history/GHSA-6rw7-vpxm-498p/expected_diff_GHSA-6rw7-vpxm-498p.json‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
[
2+
{
3+
"summary": {
4+
"old": "qs's arrayLimit bypass in its bracket notation allows DoS via memory exhaustion\n### Summary\n\nThe `arrayLimit` option in qs does not enforce limits for bracket notation (`a[]=1&a[]=2`), allowing attackers to cause denial-of-service via memory exhaustion. Applications using `arrayLimit` for DoS protection are vulnerable.\n\n### Details\n\nThe `arrayLimit` option only checks limits for indexed notation (`a[0]=1&a[1]=2`) but completely bypasses it for bracket notation (`a[]=1&a[]=2`).\n\n**Vulnerable code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Working code** (`lib/parse.js:175`):\n```javascript\nelse if (index <= options.arrayLimit) { // Limit checked here\n obj = [];\n obj[index] = leaf;\n}\n```\n\nThe bracket notation handler at line 159 uses `utils.combine([], leaf)` without validating against `options.arrayLimit`, while indexed notation at line 175 checks `index <= options.arrayLimit` before creating arrays.\n\n### PoC\n\n**Test 1 - Basic bypass:**\n```bash\nnpm install qs\n```\n\n```javascript\nconst qs = require('qs');\nconst result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5&a[]=6', { arrayLimit: 5 });\nconsole.log(result.a.length); // Output: 6 (should be max 5)\n```\n\n**Test 2 - DoS demonstration:**\n```javascript\nconst qs = require('qs');\nconst attack = 'a[]=' + Array(10000).fill('x').join('&a[]=');\nconst result = qs.parse(attack, { arrayLimit: 100 });\nconsole.log(result.a.length); // Output: 10000 (should be max 100)\n```\n\n**Configuration:**\n- `arrayLimit: 5` (test 1) or `arrayLimit: 100` (test 2)\n- Use bracket notation: `a[]=value` (not indexed `a[0]=value`)\n\n### Impact\n\nDenial of Service via memory exhaustion. Affects applications using `qs.parse()` with user-controlled input and `arrayLimit` for protection.\n\n**Attack scenario:**\n1. Attacker sends HTTP request: `GET /api/search?filters[]=x&filters[]=x&...&filters[]=x` (100,000+ times)\n2. Application parses with `qs.parse(query, { arrayLimit: 100 })`\n3. qs ignores limit, parses all 100,000 elements into array\n4. Server memory exhausted \u2192 application crashes or becomes unresponsive\n5. Service unavailable for all users\n\n**Real-world impact:**\n- Single malicious request can crash server\n- No authentication required\n- Easy to automate and scale\n- Affects any endpoint parsing query strings with bracket notation\n\n### Suggested Fix\n\nAdd `arrayLimit` validation to the bracket notation handler. The code already calculates `currentArrayLength` at line 147-151, but it's not used in the bracket notation handler at line 159.\n\n**Current code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))\n ? []\n : utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Fixed code**:\n```javascript\nif (root === '[]' && options.parseArrays) {\n // Use currentArrayLength already calculated at line 147-151\n if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) {\n throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.');\n }\n \n // If limit exceeded and not throwing, convert to object (consistent with indexed notation behavior)\n if (currentArrayLength >= options.arrayLimit) {\n obj = options.plainObjects ? { __proto__: null } : {};\n obj[currentArrayLength] = leaf;\n } else {\n obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))\n ? []\n : utils.combine([], leaf);\n }\n}\n```\n\nThis makes bracket notation behaviour consistent with indexed notation, enforcing `arrayLimit` and converting to object when limit is exceeded (per README documentation).",
5+
"new": "qs's arrayLimit bypass in its bracket notation allows DoS via memory exhaustion\n### Summary\n\nThe `arrayLimit` option in qs did not enforce limits for bracket notation (`a[]=1&a[]=2`), only for indexed notation (`a[0]=1`). This is a consistency bug; `arrayLimit` should apply uniformly across all array notations.\n\n**Note:** The default `parameterLimit` of 1000 effectively mitigates the DoS scenario originally described. With default options, bracket notation cannot produce arrays larger than `parameterLimit` regardless of `arrayLimit`, because each `a[]=value` consumes one parameter slot. The severity has been reduced accordingly.\n\n### Details\n\nThe `arrayLimit` option only checked limits for indexed notation (`a[0]=1&a[1]=2`) but did not enforce it for bracket notation (`a[]=1&a[]=2`).\n\n**Vulnerable code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Working code** (`lib/parse.js:175`):\n```javascript\nelse if (index <= options.arrayLimit) { // Limit checked here\n obj = [];\n obj[index] = leaf;\n}\n```\n\nThe bracket notation handler at line 159 uses `utils.combine([], leaf)` without validating against `options.arrayLimit`, while indexed notation at line 175 checks `index <= options.arrayLimit` before creating arrays.\n\n### PoC\n\n```javascript\nconst qs = require('qs');\nconst result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5&a[]=6', { arrayLimit: 5 });\nconsole.log(result.a.length); // Output: 6 (should be max 5)\n```\n\n**Note on parameterLimit interaction:** The original advisory's \"DoS demonstration\" claimed a length of 10,000, but `parameterLimit` (default: 1000) caps parsing to 1,000 parameters. With default options, the actual output is 1,000, not 10,000.\n\n### Impact\n\nConsistency bug in `arrayLimit` enforcement. With default `parameterLimit`, the practical DoS risk is negligible since `parameterLimit` already caps the total number of parsed parameters (and thus array elements from bracket notation). The risk increases only when `parameterLimit` is explicitly set to a very high value."
6+
}
7+
},
8+
{
9+
"severities": {
10+
"added": [
11+
{
12+
"scoring_elements": "",
13+
"system": "generic_textual",
14+
"value": "MODERATE"
15+
},
16+
{
17+
"scoring_elements": "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L",
18+
"system": "cvssv3.1",
19+
"value": "3.7"
20+
},
21+
{
22+
"scoring_elements": "CVSS:4.0/AV:N/AC:L/AT:P/PR:N/UI:N/VC:N/VI:N/VA:L/SC:N/SI:N/SA:L",
23+
"system": "cvssv4",
24+
"value": "6.3"
25+
}
26+
],
27+
"removed": [
28+
{
29+
"scoring_elements": "",
30+
"system": "generic_textual",
31+
"value": "HIGH"
32+
},
33+
{
34+
"scoring_elements": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H",
35+
"system": "cvssv3.1",
36+
"value": "7.5"
37+
},
38+
{
39+
"scoring_elements": "CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N",
40+
"system": "cvssv4",
41+
"value": "8.7"
42+
}
43+
]
44+
}
45+
}
46+
]

‎vulnerabilities/tests/test_data/advisory_history/GHSA-6rw7-vpxm-498p/expected_formatted_diff_GHSA-6rw7-vpxm-498p.json‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
[
2+
{
3+
"Summary": {
4+
"old": "qs's arrayLimit bypass in its bracket notation allows DoS via memory exhaustion\n### Summary\n\nThe `arrayLimit` option in qs does not enforce limits for bracket notation (`a[]=1&a[]=2`), allowing attackers to cause denial-of-service via memory exhaustion. Applications using `arrayLimit` for DoS protection are vulnerable.\n\n### Details\n\nThe `arrayLimit` option only checks limits for indexed notation (`a[0]=1&a[1]=2`) but completely bypasses it for bracket notation (`a[]=1&a[]=2`).\n\n**Vulnerable code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Working code** (`lib/parse.js:175`):\n```javascript\nelse if (index <= options.arrayLimit) { // Limit checked here\n obj = [];\n obj[index] = leaf;\n}\n```\n\nThe bracket notation handler at line 159 uses `utils.combine([], leaf)` without validating against `options.arrayLimit`, while indexed notation at line 175 checks `index <= options.arrayLimit` before creating arrays.\n\n### PoC\n\n**Test 1 - Basic bypass:**\n```bash\nnpm install qs\n```\n\n```javascript\nconst qs = require('qs');\nconst result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5&a[]=6', { arrayLimit: 5 });\nconsole.log(result.a.length); // Output: 6 (should be max 5)\n```\n\n**Test 2 - DoS demonstration:**\n```javascript\nconst qs = require('qs');\nconst attack = 'a[]=' + Array(10000).fill('x').join('&a[]=');\nconst result = qs.parse(attack, { arrayLimit: 100 });\nconsole.log(result.a.length); // Output: 10000 (should be max 100)\n```\n\n**Configuration:**\n- `arrayLimit: 5` (test 1) or `arrayLimit: 100` (test 2)\n- Use bracket notation: `a[]=value` (not indexed `a[0]=value`)\n\n### Impact\n\nDenial of Service via memory exhaustion. Affects applications using `qs.parse()` with user-controlled input and `arrayLimit` for protection.\n\n**Attack scenario:**\n1. Attacker sends HTTP request: `GET /api/search?filters[]=x&filters[]=x&...&filters[]=x` (100,000+ times)\n2. Application parses with `qs.parse(query, { arrayLimit: 100 })`\n3. qs ignores limit, parses all 100,000 elements into array\n4. Server memory exhausted \u2192 application crashes or becomes unresponsive\n5. Service unavailable for all users\n\n**Real-world impact:**\n- Single malicious request can crash server\n- No authentication required\n- Easy to automate and scale\n- Affects any endpoint parsing query strings with bracket notation\n\n### Suggested Fix\n\nAdd `arrayLimit` validation to the bracket notation handler. The code already calculates `currentArrayLength` at line 147-151, but it's not used in the bracket notation handler at line 159.\n\n**Current code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))\n ? []\n : utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Fixed code**:\n```javascript\nif (root === '[]' && options.parseArrays) {\n // Use currentArrayLength already calculated at line 147-151\n if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) {\n throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.');\n }\n \n // If limit exceeded and not throwing, convert to object (consistent with indexed notation behavior)\n if (currentArrayLength >= options.arrayLimit) {\n obj = options.plainObjects ? { __proto__: null } : {};\n obj[currentArrayLength] = leaf;\n } else {\n obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))\n ? []\n : utils.combine([], leaf);\n }\n}\n```\n\nThis makes bracket notation behaviour consistent with indexed notation, enforcing `arrayLimit` and converting to object when limit is exceeded (per README documentation).",
5+
"new": "qs's arrayLimit bypass in its bracket notation allows DoS via memory exhaustion\n### Summary\n\nThe `arrayLimit` option in qs did not enforce limits for bracket notation (`a[]=1&a[]=2`), only for indexed notation (`a[0]=1`). This is a consistency bug; `arrayLimit` should apply uniformly across all array notations.\n\n**Note:** The default `parameterLimit` of 1000 effectively mitigates the DoS scenario originally described. With default options, bracket notation cannot produce arrays larger than `parameterLimit` regardless of `arrayLimit`, because each `a[]=value` consumes one parameter slot. The severity has been reduced accordingly.\n\n### Details\n\nThe `arrayLimit` option only checked limits for indexed notation (`a[0]=1&a[1]=2`) but did not enforce it for bracket notation (`a[]=1&a[]=2`).\n\n**Vulnerable code** (`lib/parse.js:159-162`):\n```javascript\nif (root === '[]' && options.parseArrays) {\n obj = utils.combine([], leaf); // No arrayLimit check\n}\n```\n\n**Working code** (`lib/parse.js:175`):\n```javascript\nelse if (index <= options.arrayLimit) { // Limit checked here\n obj = [];\n obj[index] = leaf;\n}\n```\n\nThe bracket notation handler at line 159 uses `utils.combine([], leaf)` without validating against `options.arrayLimit`, while indexed notation at line 175 checks `index <= options.arrayLimit` before creating arrays.\n\n### PoC\n\n```javascript\nconst qs = require('qs');\nconst result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5&a[]=6', { arrayLimit: 5 });\nconsole.log(result.a.length); // Output: 6 (should be max 5)\n```\n\n**Note on parameterLimit interaction:** The original advisory's \"DoS demonstration\" claimed a length of 10,000, but `parameterLimit` (default: 1000) caps parsing to 1,000 parameters. With default options, the actual output is 1,000, not 10,000.\n\n### Impact\n\nConsistency bug in `arrayLimit` enforcement. With default `parameterLimit`, the practical DoS risk is negligible since `parameterLimit` already caps the total number of parsed parameters (and thus array elements from bracket notation). The risk increases only when `parameterLimit` is explicitly set to a very high value."
6+
}
7+
},
8+
{
9+
"Severities": {
10+
"added": [
11+
{
12+
"header": "Severity",
13+
"attributes": [
14+
[
15+
"System",
16+
"Generic Textual"
17+
],
18+
[
19+
"Value",
20+
"MODERATE"
21+
]
22+
]
23+
},
24+
{
25+
"header": "Severity",
26+
"attributes": [
27+
[
28+
"System",
29+
"CVSSV3.1"
30+
],
31+
[
32+
"Value",
33+
"3.7"
34+
],
35+
[
36+
"Elements",
37+
"CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L"
38+
]
39+
]
40+
},
41+
{
42+
"header": "Severity",
43+
"attributes": [
44+
[
45+
"System",
46+
"CVSSV4"
47+
],
48+
[
49+
"Value",
50+
"6.3"
51+
],
52+
[
53+
"Elements",
54+
"CVSS:4.0/AV:N/AC:L/AT:P/PR:N/UI:N/VC:N/VI:N/VA:L/SC:N/SI:N/SA:L"
55+
]
56+
]
57+
}
58+
],
59+
"removed": [
60+
{
61+
"header": "Severity",
62+
"attributes": [
63+
[
64+
"System",
65+
"Generic Textual"
66+
],
67+
[
68+
"Value",
69+
"HIGH"
70+
]
71+
]
72+
},
73+
{
74+
"header": "Severity",
75+
"attributes": [
76+
[
77+
"System",
78+
"CVSSV3.1"
79+
],
80+
[
81+
"Value",
82+
"7.5"
83+
],
84+
[
85+
"Elements",
86+
"CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H"
87+
]
88+
]
89+
},
90+
{
91+
"header": "Severity",
92+
"attributes": [
93+
[
94+
"System",
95+
"CVSSV4"
96+
],
97+
[
98+
"Value",
99+
"8.7"
100+
],
101+
[
102+
"Elements",
103+
"CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N"
104+
]
105+
]
106+
}
107+
]
108+
}
109+
}
110+
]

0 commit comments

Comments
 (0)