security(bvm): gate SETMAXATTRIBUTE / CHANGEMAXATTRIBUTE — clicker brick vector#300
Merged
Merged
Conversation
…ick vector Sibling-asymmetry gate added. BVM_SETATTRIBUTE / BVM_CHANGEATTRIBUTE have been BVM_RequirePrivileged-gated since the original privilege sweep -- their HealthStat branch falls through to KillActor(Actor, Null) and a clicker exploit could one-shot the player. The MAX-counterparts sit one function block below in ScriptingCommands.bb and were never gated. Discovered during PR #299 (P_StatUpdate detail page) recon when auditing the four SET/CHANGE/SETMAX/CHANGEMAX mutators for the doc's importance-check catalogue. Threat: non-priv NPC's Examine/Trade/RightClick/ItemScript can call: SetMaxAttribute(player, "Health", 1) -> permanent max HP = 1, next damage tick kills SetMaxAttribute(player, "Speed", 0) -> player can't move SetMaxAttribute(player, "Energy", 0) -> player can't cast spells ChangeMaxAttribute(player, "Health", -big%) -> same brick via the relative-mutation path For Examine/Trade/RightClick/ItemScript spawns, ServerNet.bb calls ThreadScript(script, method, Handle(clicker), Handle(NPC)), so SI\AI = Handle(clicker). A self-or-priv gate on Param1 would let SetMaxAttribute(clicker_handle, "Health", 1) reach the lethal path. Full-priv (BVM_RequirePrivileged) is the correct choice -- same reasoning as the SET/CHANGE pair (CLAUDE.md "BVM clicker-handle trap"). Test coverage in src/Tests/Modules/BVMPrivilegeGateTest.bb: - testSetMaxAttributeGateBlocksArbitraryTarget (Param1 != SI\AI) - testSetMaxAttributeGateBlocksBrickingOwnAITarget (Param1 == SI\AI, the clicker shape that defeats self-or-priv) - testSetMaxAttributeGateBlocksBrickingOwnContextTarget (Param1 == SI\AIContext, NPC-self-spawn shape) - testSetMaxAttributeGatePassesForPrivileged - testChangeMaxAttributeGateBlocksArbitraryTarget - testChangeMaxAttributeGateBlocksBrickingOwnAITarget - testChangeMaxAttributeGatePassesForPrivileged Plus updated the test file header (was "Seven BVM functions had effects identical to already-gated peers" -- now nine, since SETMAX and CHANGEMAX join the gated set). CLAUDE.md "Pairs to keep in lockstep" list extended: "...BVM_SETMAXATTRIBUTE / BVM_CHANGEMAXATTRIBUTE (no KillActor fall-through but still a brick vector...). All five families use BVM_RequirePrivileged()..." Compile-verified clean across all 5 engine targets. All 26 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 27, 2026
CoreyRDean
added a commit
that referenced
this pull request
May 27, 2026
docs/modules/scripting.md (the already-existing runtime-half overview) references docs/modules/scriptingcommands.md as the implementation- half companion -- but the file didn't exist. Dangling link surfaced during iteration #38 recon. Wrote the module-level overview for ScriptingCommands.bb (~3300 lines, 222 BVM_* functions): * File-structure section table (~50-3300 grouped by theme: privilege helpers / actor lifecycle / items / spells / attributes / party / output / persistence / UDP). Refresh trigger when reorganization happens; navigation aid, not strict spec. * Privilege gating section consolidates the four CLAUDE.md gate categories, the clicker-handle trap (`SI\AI = Handle(clicker)` for Examine/Trade/RightClick/ItemScript), and the full currently-gated brick-vector cluster (11 functions with their threat shapes). * Dead-API surface (BVM_SETOWNER + BVM_SCENERYOWNER) -- references PR #297's stack-balance sentinel fix and the opcode-stability rationale for keeping the contract entries alive. * Float / integer hardening overview -- ClampWorldCoord# / ClampSaneFloat# and the bounds-check-before-array-index pattern. * Handle-Null discipline -- the canonical entry pattern that every BVM body must follow (Object.X(handle) returns Null for stale handles). * "Adding a new BVM function" three-file-lockstep procedure (ScriptingCommands.bb impl + RC_Standard_Invoker.bb contract + dispatch Case + RC_Standard.bcs compile-time twin), the alphabetical-opcode-shift gotcha, and the privilege-gate decision tree. * Notable historical hardening table cross-referencing PRs #260, #237-#239, #246-#248, #233/#234, #300, #301, #304. * Related-modules section linking back to scripting.md / bvm-reference.md / RC_Standard_Invoker.bb / RC_Standard.bcs / ServerNet.bb / BVMPrivilegeGateTest.bb / CLAUDE.md. Closes the dangling-link gap; future scripting.md readers now have a landing page for the implementation half. rc_standard_invoker.md is still a dangling link from scripting.md -- deferred to a separate iteration (it's more arcane; the BVM-reference auto-gen already covers the user-facing API surface). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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
Sibling-asymmetry gate added.
BVM_SETATTRIBUTE/BVM_CHANGEATTRIBUTEhave beenBVM_RequirePrivileged-gated since the original privilege sweep — theirHealthStatbranch falls through toKillActor(Actor, Null)and a clicker exploit could one-shot the player. The MAX-counterparts (BVM_SETMAXATTRIBUTE,BVM_CHANGEMAXATTRIBUTE) sit one function block below in ScriptingCommands.bb and were never gated. Discovered during PR #299 (P_StatUpdate detail page) recon when cataloguing the four SET/CHANGE/SETMAX/CHANGEMAX importance dispatchers.Threat
Non-priv NPC's Examine/Trade/RightClick/ItemScript can call:
SetMaxAttribute(player, "Health", 1)SetMaxAttribute(player, "Speed", 0)SetMaxAttribute(player, "Energy", 0)ChangeMaxAttribute(player, "Health", -big%)Why full-priv (not self-or-priv)
For Examine/Trade/RightClick/ItemScript spawns,
ServerNet.bbcallsThreadScript(script, method, Handle(clicker), Handle(NPC)), soSI\AI = Handle(clicker). ABVM_RequireSelfOrPrivileged(Param1)gate would letSetMaxAttribute(clicker_handle, "Health", 1)reach the lethal path becauseParam1 = SI\AI. Full-priv (BVM_RequirePrivileged) is the correct choice — same reasoning as the SET/CHANGE pair (see CLAUDE.md "BVM clicker-handle trap" and the audit-comment cluster at ScriptingCommands.bb:2209-2222).Test coverage
7 new tests in
src/Tests/Modules/BVMPrivilegeGateTest.bb:testSetMaxAttributeGateBlocksArbitraryTarget(Param1 ≠ SI\AI)testSetMaxAttributeGateBlocksBrickingOwnAITarget(Param1 == SI\AI — the clicker shape that defeats self-or-priv)testSetMaxAttributeGateBlocksBrickingOwnContextTarget(Param1 == SI\AIContext)testSetMaxAttributeGatePassesForPrivilegedtestChangeMaxAttributeGateBlocksArbitraryTargettestChangeMaxAttributeGateBlocksBrickingOwnAITargettestChangeMaxAttributeGatePassesForPrivilegedTest file header updated from "Seven BVM functions" to "Nine BVM functions" (the SETMAX/CHANGEMAX pair joins the gated set).
CLAUDE.md update
"Pairs to keep in lockstep" list at line 246 extended:
Test plan
compile.bat -tclean across all 5 engine targets (Server + Client + GUE + Project Manager + Loom)test.bat BVMPrivilegeGateTest— all gate cases passtest.bat— full suite: 26 of 26 passdata/currently callsSetMaxAttribute/ChangeMaxAttributefrom a non-priv path (the gate would log + return False, but legitimate priv-spawn calls — quest reward scripts that adjust max-Health, etc. — keep working unchanged)🤖 Generated with Claude Code