-
Notifications
You must be signed in to change notification settings - Fork 2
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
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,38 +3,13 @@ | |
| * SPDX-License-Identifier: MIT | ||
| */ | ||
|
|
||
| import vm from 'node:vm' | ||
| import { type Request, type Response, type NextFunction } from 'express' | ||
| // @ts-expect-error FIXME due to non-existing type definitions for notevil | ||
| import { eval as safeEval } from 'notevil' | ||
| import { type Request, type Response } from 'express' | ||
|
|
||
| import * as challengeUtils from '../lib/challengeUtils' | ||
| import { challenges } from '../data/datacache' | ||
| import * as security from '../lib/insecurity' | ||
| import * as utils from '../lib/utils' | ||
|
|
||
| export function b2bOrder () { | ||
| return ({ body }: Request, res: Response, next: NextFunction) => { | ||
| if (utils.isChallengeEnabled(challenges.rceChallenge) || utils.isChallengeEnabled(challenges.rceOccupyChallenge)) { | ||
| const orderLinesData = body.orderLinesData || '' | ||
| try { | ||
| const sandbox = { safeEval, orderLinesData } | ||
| vm.createContext(sandbox) | ||
| vm.runInContext('safeEval(orderLinesData)', sandbox, { timeout: 2000 }) | ||
| res.json({ cid: body.cid, orderNo: uniqueOrderNumber(), paymentDue: dateTwoWeeksFromNow() }) | ||
| } catch (err) { | ||
| if (utils.getErrorMessage(err).match(/Script execution timed out.*/) != null) { | ||
| challengeUtils.solveIf(challenges.rceOccupyChallenge, () => { return true }) | ||
| res.status(503) | ||
| next(new Error('Sorry, we are temporarily not available! Please try again later.')) | ||
| } else { | ||
| challengeUtils.solveIf(challenges.rceChallenge, () => { return utils.getErrorMessage(err) === 'Infinite loop detected - reached max iterations' }) | ||
| next(err) | ||
| } | ||
| } | ||
| } else { | ||
| res.json({ cid: body.cid, orderNo: uniqueOrderNumber(), paymentDue: dateTwoWeeksFromNow() }) | ||
| } | ||
| return ({ body }: Request, res: Response) => { | ||
| res.json({ cid: body.cid, orderNo: uniqueOrderNumber(), paymentDue: dateTwoWeeksFromNow() }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Enabled RCE challenges become unsolvable When either RCE challenge is enabled, Learn moreThese challenges are loaded into the score board unless their environment restrictions or safety mode disable them. Their only completion path was removed from 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. Was this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed that Full retirement would mean editing |
||
| } | ||
|
|
||
| function uniqueOrderNumber () { | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Obsolete interpreter dependency remains
notevilhas no remaining code imports but stays in the dependency manifest and lock file. Remove it with the retired evaluation path.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
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,
notevilis needed again, and dropping it now would also churnpackage-lock.jsonin a PR whose direction is still open. Happy to remove the dependency in the same follow-up that retires the challenge metadata.