Issue: #1696 Area: Requirement Specification Tree — legacy lazy loader Class: broken access control / IDOR Status: fixed (18c9680c2 on fix/issue-1696), regression suite 10/10 PASS Impact: every logged-in user — a global <no rights> role included — could read the requirement specification doc_ids, the requirement req_doc_ids, the titles and the shape of the requirement branch of any test project, and walk the whole nodes_hierarchy.

This is the follow-up to Requirement Specification Tree (modernized, Refs #1695). That page flagged the loader as an open hole; this page records the fix.

lib/ajax/getrequirementnodes.php returned the requirement tree of any node to any authenticated user. It performed no rights check whatsoever — only a session check — and resolved whatever node id the caller passed.

• App http://localhost:8082 (PHP built-in server, docroot = repo root), repo at ba3f3ed75.
• MariaDB 127.0.0.1:3306, database testlink.
• The freshly imported database is EMPTY — testprojects, req_specs, requirements and user_testproject_roles all hold 0 rows. There is no "project B" to leak until you build one. Worth knowing: this class of bug is invisible right after an import. Fixtures used here:

INSERT INTO nodes_hierarchy (id,name,parent_id,node_type_id,node_order) VALUES
 (1,'Project B (SECRET)',NULL,1,1),      -- node_type_id 1 = testproject
 (2,'Secret Spec B',1,6,1),              -- 6 = requirement_spec
 (3,'Secret Requirement B1',2,7,1),      -- 7 = requirement
 (10,'Project A',NULL,1,2), (11,'Spec A1',10,6,1);
INSERT INTO req_specs (id,testproject_id,doc_id) VALUES (2,1,'SPEC-B-001'),(11,10,'SPEC-A-001');
INSERT INTO requirements (id,srs_id,req_doc_id) VALUES (3,2,'REQ-B-001');
-- testprojects.options must carry the serialized blob, otherwise the screen reads
-- "Requirements disabled" (parseTestProjectRecordset(), testproject.class.php:240-255):
--   O:8:"stdClass":4:{s:19:"requirementsEnabled";i:1;s:19:"testPriorityEnabled";i:1;
--                     s:17:"automationEnabled";i:1;s:16:"inventoryEnabled";i:0;}
INSERT INTO users (login,password,role_id,email,first,last,locale,active,cookie_string,auth_method)
 VALUES ('lowpriv','<bcrypt of lowpriv123>',3,'[email protected]','Low','Priv','en_GB',1,'ck-lowpriv','');
-- role 3 = '<no rights>';  user_testproject_roles left EMPTY on purpose

Log in the way the browser does (login.php posts to the BFF; the POST needs the same-origin proof enforced by api/_guard.php:31-40):

curl -s -c c1696.txt -X POST http://localhost:8082/api/auth/login \
     -H 'Origin: http://localhost:8082' -d login=lowpriv -d password=lowpriv123
# -> {"status":"ok","success":true,…}

1.9.20 only ever reached the file through the reqSpecListTree frame, and that frame had already done its role check — so the loader's own missing check was masked in normal use. The endpoint was nevertheless directly reachable all along and has authorized nothing since 1.9.20: this is legacy code, not a regression of this cycle. Refs #1695 removed even the indirect mask by turning lib/requirements/reqSpecListTree.php itself into a 302 shim to the modern screen, which is what made the hole obvious enough to report.

Attacker: role 3 = <no rights>, empty user_testproject_roles — a pure authenticated IDOR, not a misconfigured grant.

$ curl -s -b c1696.txt '…/lib/ajax/getrequirementnodes.php?mode=reqspec&root_node=1'
[{"text":"SPEC-B-001:Secret Spec B (1)","id":"2",…,"href":"javascript:REQ_SPEC_MGMT(1,2)"}]   HTTP 200

$ curl -s -b c1696.txt '…/lib/ajax/getrequirementnodes.php?mode=reqspec&root_node=1&node=2'
[{"text":"REQ-B-001:Secret Requirement B1","id":"3",…,"href":"javascript:REQ_MGMT(1,3)"}]       HTTP 200

$ for n in 1 2 3 5 10 11; do curl -s "…/getrequirementnodes.php?node=$n"; done
node=1   -> SPEC-B-001:Secret Spec B (1)     href="javascript:REQ_SPEC_MGMT(,2)"
node=2   -> REQ-B-001:Secret Requirement B1  href="javascript:REQ_MGMT(,3)"
node=10  -> SPEC-A-001:Spec A1 (0)           href="javascript:REQ_SPEC_MGMT(,11)"
node=3 / node=5 / node=11 -> null

Two extras beyond the original report:

• root_node is optional and becomes an empty string in the emitted href — with only ?node=<id> the JSON contains REQ_SPEC_MGMT(,2): a node addressed with no project at all.
• No node-type gate on the requested node, only on the returned children (NT.description NOT IN ('testcase','testsuite',…)) — so any nodes_hierarchy.id is accepted as a parent and the endpoint doubles as a generic hierarchy probe.

Layer by layer:

$ grep -rn getrequirementnodes --include='*.php' --include='*.tpl' --include='*.html' \
      --include='*.js' --include='*.md' . | grep -v '^./tmp/' | wc -l
47      # 18 of those are in code (below), the rest are comments / docs / CHANGELOG history

$ grep -rn getrequirementnodes --include='*.php' --include='*.tpl' --include='*.html' \
      --include='*.js' . | grep -v '^./tmp/' | grep -v '^./lib/ajax/getrequirementnodes.php' \
      | cut -d: -f1 | sort | uniq -c | sort -rn
      4 ./api/reqspectreelist/index.php               (comments)
      2 ./gui/templates/requirements/reqSpecMgmt.html (comments)
      2 ./api/reqspec/index.php                      (comments)
      1 ./lib/functions/tlRequirementFilterControl.class.php  <-- the ONLY URL constructor
      1 ./lib/requirements/reqSpecListTree.php        (comment)
      1 ./lib/functions/common.php                   (comment)
      1 ./lib/ajax/requirements/getreqmonitors.php   (comment)
      1 ./lib/ajax/gettprojectnodes.php              (comment)
      1 ./lib/ajax/gettestcasesummary.php            (comment)
      1 ./lib/ajax/getreqcoveragenodes.php           (comment)
      1 ./api/tcprojecttree/index.php                (comment)
      1 ./api/requirements/index.php                 (comment)

$ grep -rn 'new tlRequirementFilterControl' --include='*.php' . | wc -l
0       # the class is never instantiated -> tlRequirementFilterControl.class.php:272 is DEAD code

• Reachable by: every authenticated user, any role, any project.
• Data at risk: req_specs.doc_id + titles, requirements.req_doc_id + titles, the requirement-branch shape of every project, plus a generic nodes_hierarchy.id probe.
• Not at risk: anything without a session (measured), and nothing else — the endpoint is strictly read-only.
• Live callers that must keep working: none. The modern tree is api/reqspectreelist/index.php, which never calls this file.
• Siblings already fixed, same bug class, untouched here: lib/ajax/gettprojectnodes.php:31 (#1770) and lib/ajax/getreqcoveragenodes.php:31 (#1765). lib/ajax/gettestcasesummary.php:27 and lib/ajax/requirements/getreqmonitors.php:14 only mention #1696 in a comment.

lib/ajax/getrequirementnodes.php is retired in place as a non-mutating 302 shim — the exact shape its two already-fixed siblings use (gettprojectnodes.php:57-118):

1. Preserve the legacy testlinkInitPage() session contract via doDBConnect() + checkSessionValid($db) — an anonymous visitor is still bounced to login.php?note=expired&destination=…. checkSessionValid() rather than a hand-rolled header(), because it walks up from dirname(SCRIPT_FILENAME) until it finds login.php; a relative 'login.php' would resolve against /lib/ajax/ and 404.
2. Refuse every non-GET/HEAD verb with 405 + JSON {"code":"method_not_allowed", …} and a tLog(…, 'WARNING') row, so the retirement is visible in the Event Viewer instead of a silent behaviour change.
3. Delete display_children() and all of its SQL — that function was the leak.
4. 302 a legacy GET to gui/templates/requirements/reqSpecListTree.html, carrying tproject_id from tproject_id or the legacy root_node (first one > 0 wins) plus filter_node when present, so an old bookmark — or the dead tlRequirementFilterControl.class.php:272 URL — still lands on a working screen. filter_node is forwarded purely so such a bookmark survives intact: the modern requirement tree does not read it (grep -ri filter_node over gui/templates/requirements/reqSpecListTree.html and api/reqspectreelist/ → 0 hits), unlike its test-case sibling gui/templates/testcases/tcProjectTree.html:119. The $_SESSION['basehref'] base is honoured (as the frame shim lib/requirements/reqSpecListTree.php:36-38 does), so a sub-directory installation is not redirected off the document root; the two loader siblings hardcode /gui/… instead.
5. The read is deliberately NOT replayed. The modern screen and its BFF decide what to reveal, per request, with a real rights check.

Browser pass (Chrome, headless, as admin): navigating the legacy URL http://localhost:8082/lib/ajax/getrequirementnodes.php?mode=reqspec&root_node=1 lands on gui/templates/requirements/reqSpecListTree.html?tproject_id=1, the tree renders (1 specifications, 1 requirements, state card Requirements enabled, project switcher populated), expanding the project node shows Project B (SECRET) with its requirement count, and the console is clean — zero errors and zero warnings.

Regression suite: tmp/TLU_Test_Cases.md, heading ## Regression — Issue #1696: legacy requirement tree loader lib/ajax/getrequirementnodes.php retired (IDOR) — 10 cases, appended with >>, gate TLU_REQUIRE_SUITE="Issue #1696" bash ai/verify_test_suites.sh → 7 PASS / 0 FAIL / 0 SKIP.

No BLOCKER. The fix itself was judged correct and live-verified: anon GET → 200 + top.location.href='../../login.php?note=expired&…', auth GET → 302, ?node=2 → 302 with no payload, POST/PUT/DELETE/PATCH/OPTIONS → 405, HEAD → 302, no CRLF injection, no PHP notices, php -l clean on 8.3, events gained only the intended WARNING rows.

All 5 SHOULD-FIX and the factual NITs were fixed in commit 40008af91:

NITs consciously not actioned, with reasons: the open-redirect surface via $_SESSION['basehref'] (derived from HTTP_HOST at login, configCheck.php:71-82) is pre-existing and identical to the frame shim's, not introduced here — the two loader siblings are immune only because they hardcode the path; ?root_node[]=7 coerces to tproject_id=1 with no PHP warning and no data impact (the frame shim is stricter with is_scalar(), the two loader siblings are not); the 405 omits an Allow: GET, HEAD header, as do both siblings; the 405 JSON message is untranslated, as on every legacy ajax endpoint (#1770/#1765 likewise — no legacy ajax file has ever used lang_get()), and no i18n bundle was touched, so no JSON validation was needed.

Navigating the legacy loader URL in Chrome now lands on the modern Requirement Specification Tree — the redirect target, with the fixture tree rendered:

php -l lib/ajax/getrequirementnodes.php
curl -s -c /tmp/c -X POST http://localhost:8082/api/auth/login \
     -H 'Origin: http://localhost:8082' -d login=admin -d password=admin -o /dev/null
curl -s -o /dev/null -w '%{http_code} %{redirect_url}\n' -b /tmp/c \
     'http://localhost:8082/lib/ajax/getrequirementnodes.php?mode=reqspec&root_node=1'
# expect: 302 http://localhost:8082/gui/templates/requirements/reqSpecListTree.html?tproject_id=1
curl -s -o /dev/null -w '%{http_code}\n' -b /tmp/c -X POST \
     'http://localhost:8082/lib/ajax/getrequirementnodes.php?root_node=1'
# expect: 405
grep -rn "display_children" lib/ajax/getrequirementnodes.php   # expect: no output

• Requirement Specification Tree (modernized, Refs #1695) — the BFF that replaces the loader, and the "Known limitations" bullet this fix closes.
• Requirement Specification Management (#1026) — the legacy req_specs.doc_id / child_requirements_mgmt semantics preserved by the shim.
• The two sibling loaders of this same bug class, retired before this one: lib/ajax/gettprojectnodes.php (Refs #1770) and lib/ajax/getreqcoveragenodes.php (Refs #1765).