Skip to content

Fix: Server-side evaluation of user-supplied orderLinesData in POST /b2b/v2/orders enables CPU-exhaustion denial of service and sandbox-escape code execution - #359

Closed
WesternConcrete wants to merge 1 commit into
developfrom
devin/1789111474-fix-b2b-eval
Closed

WesternConcrete wants to merge 1 commit into
developfrom
devin/1789111474-fix-b2b-eval

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Sep 11, 2026

Copy link
Copy Markdown

Description

Finding: Server-side evaluation of user-supplied orderLinesData in POST /b2b/v2/orders enables CPU-exhaustion denial of service and sandbox-escape code execution
Repo: COG-GTM/juice-shop

Fix approach: routes/b2bOrder.ts no longer passes body.orderLinesData to the notevil interpreter inside vm.runInContext, so the endpoint just acknowledges the order instead of interpreting attacker-supplied JavaScript.

Any authenticated customer could previously post {"cid":"x","orderLinesData":"while(true){}"} and pin the single-threaded event loop for the full 2000 ms timeout (repeatable/concurrent), with any notevil sandbox bypass escalating to code execution in the server process.

Accompanying test changes:

  • test/api/b2b-order.test.ts now asserts that endless-loop and sandbox-breakout payloads are accepted as inert data (HTTP 200) instead of being evaluated,
  • test/cypress/e2e/b2bOrder.spec.ts is removed because every case in it drove the removed evaluation path.

Maintainer note: this retires the attack surface behind the rceChallenge and rceOccupyChallenge training challenges. Their metadata (data/static/challenges.yml, config.schema.yml, config/fbctf.yml, lib/antiCheat.ts) is left untouched deliberately — deciding between keeping the intentional vulnerability and retiring/redesigning those challenges is a maintainer call.

Verified: npx eslint routes/b2bOrder.ts test/api/b2b-order.test.ts test/server/b2bOrderSpec.ts, npm run test:server (252 passing), npx tsx --test test/api/b2b-order.test.ts (all B2B cases passing).

Resolved or fixed issue: none

AI Tool Disclosure

  • My contribution does not include any AI-generated content
  • My contribution includes AI-generated content, as disclosed below:
    • AI Tools: Devin
    • LLMs and versions: Devin (Cognition)
    • Prompts: Daily security sweep of the repository; substantiate findings against code and open a remediation PR for the B2B order evaluation finding.

Affirmation

Link to Devin session: https://app.devin.ai/sessions/5070f67d91384c4eb78cf128ba4b2500
Open in Devin Desktop: https://app.devin.ai/desktop/session/5070f67d91384c4eb78cf128ba4b2500?variant=devin
Requested by: @WesternConcrete


Devin Review

POST /b2b/v2/orders no longer interprets orderLinesData through notevil
inside a vm context, removing the CPU-exhaustion and sandbox-escape
surface. Update API tests and drop the e2e specs that depended on the
evaluation.

Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Wes Convery <2wconvery@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment thread routes/b2bOrder.ts
res.json({ cid: body.cid, orderNo: uniqueOrderNumber(), paymentDue: dateTwoWeeksFromNow() })
}
return ({ body }: Request, res: Response) => {
res.json({ cid: body.cid, orderNo: uniqueOrderNumber(), paymentDue: dateTwoWeeksFromNow() })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Enabled RCE challenges become unsolvable

When either RCE challenge is enabled, b2bOrder accepts every payload without invoking its solver. The challenge definitions remain active, so users can never complete them.

Learn more

These challenges are loaded into the score board unless their environment restrictions or safety mode disable them. Their only completion path was removed from b2bOrder, and the codebase contains no other solver calls for either challenge. The endpoint therefore advertises challenges that no runtime action can solve.

Example: On a normal local installation, a player submits the documented infinite-loop payload for “Blocked RCE DoS.” The endpoint returns 200, but the challenge remains unsolved forever.

Recommended fix: Retire both challenges completely from metadata, configuration, anti-cheat mappings, tests, and related model keys, or redesign safe completion conditions. If they remain enabled, add corresponding E2E coverage as required for modified challenges.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that rceChallenge / rceOccupyChallenge become unsolvable — that is the unavoidable consequence of removing server-side evaluation, and it is flagged as a maintainer decision in the PR description rather than something I retired unilaterally.

Full retirement would mean editing data/static/challenges.yml, config.schema.yml, config/fbctf.yml, lib/antiCheat.ts, models/challenge.ts keys, test/server/b2bOrderSpec.ts, and the public documentation/solutions — i.e. removing training content, not fixing a bug. The two options are: keep the intentional vulnerability (close this PR), or approve the full retirement/redesign, which I can do in a follow-up on request.

Comment on lines -1 to -63
describe('/b2b/v2/order', () => {
describe('challenge "rce"', () => {
it('an infinite loop deserialization payload should not bring down the server', () => {
cy.task('isDocker').then((isDocker) => {
if (!isDocker) {
cy.login({ email: 'admin', password: 'admin123' })

cy.window().then(async () => {
const response = await fetch(
`${Cypress.config('baseUrl')}/b2b/v2/orders/`,
{
method: 'POST',
cache: 'no-cache',
headers: {
'Content-type': 'application/json',
Authorization: `Bearer ${localStorage.getItem('token')}`
},
body: JSON.stringify({
orderLinesData: '(function dos() { while(true); })()'
})
}
)
if (response.status === 500) {
console.log('Success')
}
})
cy.expectChallengeSolved({ challenge: 'Blocked RCE DoS' })
}
})
})
})

describe('challenge "rceOccupy"', () => {
it('should be possible to cause request timeout using a recursive regular expression payload', () => {
cy.task('isDocker').then((isDocker) => {
if (!isDocker) {
cy.login({ email: 'admin', password: 'admin123' })
cy.window().then(async () => {
const response = await fetch(
`${Cypress.config('baseUrl')}/b2b/v2/orders/`,
{
method: 'POST',
cache: 'no-cache',
headers: {
'Content-type': 'application/json',
Authorization: `Bearer ${localStorage.getItem('token')}`
},
body: JSON.stringify({
orderLinesData:
"/((a+)+)b/.test('aaaaaaaaaaaaaaaaaaaaaaaaaaaaa')"
})
}
)
if (response.status === 503) {
console.log('Success')
}
})
cy.expectChallengeSolved({ challenge: 'Successful RCE DoS' })
}
})
})
})
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Required challenge checks are missing

The repository requires RSN verification for challenge-related changes and E2E coverage for modified challenges. The stated checks omit RSN, and this change deletes the relevant E2E suite.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm run rsn passes on this branch ("All codefix files match the locked state") — this change does not touch any vuln-code-snippet markers or data/static/codefixes/ files, so the RSN cache is unaffected. I'll add that to the verification list.

The deleted e2e suite only exercised the removed evaluation path (Blocked RCE DoS / Successful RCE DoS), so there is nothing left to assert there; replacement e2e coverage only makes sense once maintainers decide whether those challenges are retired or redesigned.

Comment thread routes/b2bOrder.ts
* SPDX-License-Identifier: MIT
*/

import vm from 'node:vm'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Obsolete interpreter dependency remains

notevil has no remaining code imports but stays in the dependency manifest and lock file. Remove it with the retired evaluation path.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberately left in place: if maintainers decide to keep these as intentional challenges, notevil is needed again, and dropping it now would also churn package-lock.json in a PR whose direction is still open. Happy to remove the dependency in the same follow-up that retires the challenge metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant