Security audit fixes — 28 issues across SDK, contracts, and infrastructure #37
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "development_security_fixes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements security fixes identified by a consolidated audit (
SECURITY_AUDIT_V2.md) that synthesized findings from 8 independent AI security audit reports, covering 58 total findings with 28 fixes implemented in this PR.25 files changed — 332 insertions, 117 deletions.
Phase 0: Fix Broken SDK (CRIT-01, CRIT-02, CRIT-03)
These are blocking bugs — the SDK literally cannot work correctly without these fixes.
credit()factory method was passinggld_tokeninstead ofspore_token. The Credit Vault contract works with SPORE, not GLD. Fixed insrc/sdk/mod.rsandsrc/sdk/credit/vault.rs.spend()method was missing thepayeeparameter and useddescriptioninstead ofmemo, not matching the contract's actual signature. Fixed insrc/sdk/credit/vault.rs,src/cli/cli/credit.rs,src/cli/cli/execute.rs,src/cli/cmd.rs.get_balancewhich doesn't exist on the contract. Changed to uselist_linesand sum balances across all lines. Fixed insrc/sdk/credit/vault.rs.Phase 1: Contract Data Integrity & Fund Safety
Vector::new()andLookupMap::new()calls used the same static prefix, causing data corruption across different users' lines. Added parameterizedStorageKeyvariants:LineIndexByOwner(AccountId),ApprovalIndexByKey(AccountId, u64). Fixed incontracts/credit/src/lib.rs.U128andPromiseOrValue<U128>per NEP-141 standard. Bad payloads now refund tokens instead of panicking. Fixed incontracts/credit/src/lib.rs.claim_refundandwithdraw_feeshad no callbacks, so failed token transfers silently lost funds. Addedon_refund_completeandon_fee_withdraw_completewith proper rollback. Fixed incontracts/dns/src/lib.rs.fund_account. Addedauthorized_relayers: UnorderedSet<AccountId>withadd_relayer/remove_relayeradmin methods. Fixed incontracts/faucet/src/lib.rs.sla_settle_periodhad no caller verification. Addedadminfield and admin-only check. Fixed incontracts/hosting/src/lib.rs.chmod 0600after writes. Fixed insrc/provision/config.rs,src/cluster_manager/cluster.rs.amount.parse().expect()in withdrawal callbacks. Replaced with safematchhandling. Fixed incontracts/sporex/src/lib.rs.Phase 2: Supply Chain & Network Security
branch = \"development\", allowing supply chain attacks. Pinned to specific commit hashes. Fixed inCargo.toml.EnvironmentFile. Fixed insrc/provision/relayer_service.rs.src/sdk/network.rs.(deposit_near * 1e24) as u128loses precision. Replaced with integer-based whole/fractional split. Fixed insrc/sdk/client.rs.--secret-keyand--mnemonicvisible in process list. Added stdin prompting when not provided as args. Fixed insrc/bin/heroledger.rs.format!(\"deployed:{}\")returned as tx hash. Changed to clearly indicate no real hash. Fixed insrc/sdk/contract_builder.rs.Debugimpl now redactsprivate_key. Fixed insrc/sdk/mod.rs.docker/Dockerfile.builder.Phase 3: Hardening & Mainnet Readiness
run_command,run_command_output, andenv_varfunctions from Rhai engine. Fixed insrc/rhai/mod.rs.admin_withdraw_usdhandadmin_withdraw_gldmethods. Fixed incontracts/sporex/src/lib.rs.contracts/credit/src/lib.rs.allow_methods(Any).allow_headers(Any)to specific[GET, POST]and[CONTENT_TYPE]. Fixed insrc/cli/relayer.rs.contracts/groups/src/lib.rs.contracts/marketplace/src/lib.rs.contracts/sporex/src/lib.rs.contracts/dns/src/lib.rs.Build & Test Verification
cargo check— passes (main crate + all features)cargo check --target wasm32-unknown-unknown— passes for all 7 modified contractscargo test— 95 passed, 0 failed, 21 ignoredLeft for Future Work
assert_one_yocto→ storage deposit calculationHashSet→ NEARUnorderedSetvault_wiperemoval from Rhaiherolib_vaultcrateReference
Full audit details in
SECURITY_AUDIT_V2.mdincluded in this PR.Test plan
cargo checkpassescargo test— 95 passed, 0 failedWIP: Security audit fixes — 28 issues across SDK, contracts, and infrastructureto Security audit fixes — 28 issues across SDK, contracts, and infrastructureSecurity audit fixes — 28 issues across SDK, contracts, and infrastructureto WIP: Security audit fixes — 28 issues across SDK, contracts, and infrastructureCI status: 2/3 passing
The bootstrap failure is because the zinit download URL on
development's workflow returns 404 (forge.ourworld.tf/api/packages/geomind_code/generic/zinit/dev/zinit-linux-amd64). This PR includes a fix (commite8538dd) that switches to a working install source, but Forgejo uses the base branch's workflow file forpull_requestevents — so the fix only takes effect after merge.Also fixed in this PR: removed broken
geomind_dev_docsgitlink (registered as submodule with no.gitmodulesentry) and scoped bootstrap triggers todevelopment/mainonly (wasbranches: ["*"]).CI status: all green (2/2)
Bootstrap workflow was disabled — it has never passed on any branch (the zinit binary package at
forge.ourworld.tf/api/packages/geomind_code/generic/zinit/devwas never published). Renamed tobootstrap.yaml.disabled. Re-enable once the zinit package is available.Also fixed: removed broken
geomind_dev_docsgitlink, scoped workflow triggers todevelopment/mainonly.Bootstrap workflow note
The bootstrap workflow is disabled in this PR because zinit binaries were never published to the Forge package registry. The build-linux CI on zinit has a bug (missing musl target).
Fix is in zinit PR #20. Once that's merged, the zinit packages will be published automatically, and the bootstrap workflow here can be re-enabled.
WIP: Security audit fixes — 28 issues across SDK, contracts, and infrastructureto Security audit fixes — 28 issues across SDK, contracts, and infrastructure@mik-tf can you resolve the conflicts @scott @lee your input is needed here.
@thabeta yes of course. One thing, Scott did a lot of changes since then, so it seems this PR is outdated. Checking to go against the current development so we have the latest state.
Closing this PR.
Why: The
developmentbranch diverged significantly after this PR was opened — ~50 commits landed the same day, including an architectural shift (relayer → gateway). By the time the conflict was flagged, roughly half the original 28 fixes had been independently resolved ondevelopment, and the CORS fix (MED-03) needed to be retargeted from the deletedrelayer.rsto the newsrc/gateway/mod.rs.Rather than resolving conflicts against a codebase that has moved this far, we ran a fresh security audit against the current
developmentHEAD. The new audit (SECURITY_AUDIT_V3.md) covers 71 findings (10 Critical, 20 High, 23 Medium, 18 Low) across all areas — contracts, gateway, SDK/CLI, Rhai engine, and infrastructure — including new attack surface in the gateway, email verification, and identity systems that did not exist at the time of the V2 audit.A new issue and PR will be opened based on the V3 audit. All V2 findings that are still open are carried forward with updated IDs in V3.
Pull request closed