security: Round 2 full-stack audit — 4 medium findings (contracts + backend + frontend)#6
Open
Godbrand0 wants to merge 1 commit into
Open
security: Round 2 full-stack audit — 4 medium findings (contracts + backend + frontend)#6Godbrand0 wants to merge 1 commit into
Godbrand0 wants to merge 1 commit into
Conversation
Audits the OpenClaw backend and Next.js frontend for the first time, plus a second pass on both Clarity contracts. Finds 4 medium-severity issues not caught in the prior contract-only review: M-1 (contract): top-up-stream allows extending an already-elapsed ACTIVE stream, silently locking top-up funds if the sender uses it on a dead stream. M-2 (backend): error-handler.ts exposes raw err.message to API clients, leaking internal file paths and SDK details on 500/502 responses. M-3 (frontend): buildClaimTx, buildClaimAllTx, and buildCancelStreamTx use PostConditionMode.Allow with zero post-conditions — wallet cannot enforce expected token amounts at signing time. M-4 (frontend): AssistantWidget interpolates raw user input directly into fetch paths without encodeURIComponent, allowing path manipulation to hit unintended API routes. No critical or high findings. All fund-conservation properties of the contracts are confirmed sound.
|
@Godbrand0 is attempting to deploy a commit to the dev_jaytee's projects Team on Vercel. A member of the Team first needs to authorize it. |
jayteemoney
added a commit
that referenced
this pull request
May 19, 2026
M-2 (backend): openclaw-service error handler forwarded raw err.message to
clients, leaking file paths, SDK versions, and upstream API details. Rewrote
to generate opaque randomUUID() reference IDs; full errors logged
server-side only.
M-3 (frontend): buildClaimTx, buildClaimAllTx, and buildCancelStreamTx used
PostConditionMode.Allow with zero post-conditions, so tampered functionArgs
would produce wallet prompts indistinguishable from legitimate ones. All
three rewritten to PostConditionMode.Deny with explicit
Pc.principal(contract).willSendLte(amount).ft(token, ftName) constraints.
Function signatures extended to take ftName and amount upper bound; all
four frontend call sites updated to pass these from
getTokenConfigByContractId() (new helper in constants.ts).
M-4 (frontend): AssistantWidget interpolated raw user input into fetch paths
without encodeURIComponent, letting slashes silently redirect requests to
different routes. Added client-side regex validation (^\\d+$ for stream IDs,
^S[A-Z0-9]{38,40}$ for Stacks addresses) plus encodeURIComponent on all four
interpolated paths.
PR #6 M-1 contract finding was rejected as a false positive - the L-10
end-block guard exists on main at stream-manager.clar:693.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds
SECURITY_REVIEW_2.md, a second security audit that covers the OpenClaw backend service and Next.js frontend for the first time, plus a fresh pass on both Clarity contracts. The prior review (SECURITY_REVIEW.md, April 2026) only audited the contracts.4 medium-severity findings, 0 critical/high.
top-up-streamaccepts top-up of an already-elapsed ACTIVE stream, locking top-up funds in an orphaned streamerror-handler.tsexposes rawerr.messageto API clients (file paths, SDK internals)buildClaimTx,buildClaimAllTx,buildCancelStreamTxusePostConditionMode.Allowwith zero post-conditionsAssistantWidgetinterpolates raw user input into fetch paths withoutencodeURIComponentAll existing contract security properties (reentrancy,
contract-callerauth, token substitution prevention, fund conservation) were re-verified and remain sound.Fixes Required Before Mainnet
(asserts! (< stacks-block-height end-block) ERR-STREAM-ENDED)totop-up-streaminstream-manager.clarmessage: err.messagewith opaque error reference UUIDs inopenclaw-service/src/middleware/error-handler.tsPc.*post-conditions tobuildClaimTx,buildClaimAllTx,buildCancelStreamTxinfrontend/src/lib/stacks.tsencodeURIComponent()and add client-side regex validation infrontend/src/components/openclaw/assistant-widget.tsxTest plan
SECURITY_REVIEW_2.mdand verify all 4 findings with code referencesclarinet checkto confirm the M-1 contract change compiles0/status,../health) to confirm M-4 fix