fix: critical security and economy correctness bugs - #152
Closed
cursor[bot] wants to merge 3 commits into
Closed
cursor[bot] wants to merge 3 commits into
cursor[bot] wants to merge 3 commits into
Conversation
PR #130 added a VM sandbox for /eval, but string-concatenated process access and Function constructor chains could still read process.env. Block constructor/__proto__/prototype identifiers, reject .constructor access, and use a null-prototype VM context. Co-authored-by: Daan Vrieling <contact@zvapor.xyz>
getMember() returns null for users outside the server. Accessing member.bannable/kickable threw an uncaught TypeError and broke the command. Co-authored-by: Daan Vrieling <contact@zvapor.xyz>
Add per-user locks before DB reads in deposit/withdraw (same pattern as /rob). Remove Math.abs balance correction that turned race-induced negatives into created money. Require an economy account before /beg applies wallet changes. Co-authored-by: Daan Vrieling <contact@zvapor.xyz>
zVapor-Dev
marked this pull request as ready for review
September 16, 2026 22:51
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
2 participants
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
Automated critical-bug inspection found four high-severity issues in recent code (primarily from PR #130 and existing economy/moderation paths). This PR applies minimal, targeted fixes with regression tests.
Bugs fixed
1. safeEval sandbox bypass (security)
/evalcould exfiltrateprocess.env(bot token, DB URI) via constructor-chain escape and string-concatenatedprocessaccess.Functionvia.constructor.constructor/__proto__/prototype, reject.constructoraccess, use null-prototype VM context.2.
/banand/kickcrash on users not in guildTypeErrorwhengetMember()returns null (user not in server)..bannable/.kickable(timeout already had this guard).3.
/depositand/withdrawbalance corruptionMath.abs()on negative wallet/bank turned race losses into money creation./rob); removedMath.abs.4.
/begghost creditsValidation
npm test— 57/57 passing (9 new regression tests)Notes
PR #135 (actions/checkout bump) was reviewed separately; no application-code issues found in that diff.