GHSA-97cv-x867-6xhm: Go/github.com/klever-io/klever-go vulnerability

Description

The VM built-in function KleverUpdateAccountPermission (registered always-active, creator.go:381-390 / core/vmconstants.go:234) rewrites an account's entire permission set. Its authorization check uses vmInput.RecipientAddr attacker-controlled instead of the authenticated vmInput.CallerAddr. The sibling handler kleverChangeOwnerAddress.go:86 uses vmInput.CallerAddr correctly, so the safe pattern exists in-repo; this handler deviates. The native transaction path (txProcess.go:833) is safe it uses tx.GetSender().

Mechanism: 1. Wrong variable: CallerAddr is never referenced in the handler; auth is contractHasValidPermission(target.GetPermissions(), RecipientAddr), which returns true if RecipientAddr is a signer with Weight >= Threshold in the target account's permissions and the permission grants UpdateAccountPermissionContractType. 2. RecipientAddr is attacker-controlled: when a contract calls a built-in via ExecuteOnDestContextWithTypedArgs (baseOps.go:1967), prepareIndirectContractCallInput (baseOps.go:2485) sets RecipientAddr = destination (contract-chosen) and CallerAddr = the calling contract. The blockchain hook (blockChainHook.go:454/467) dispatches on input.Function and passes the input through unchanged; no guard forces RecipientAddr == CallerAddr and there is no SC-destination validation on this path. 3. Self-signer default satisfies the check: createDefaultOwnerPermission (accounts.go:1848) makes an account its own signer (weight 1, threshold 1, Owner type), and CheckPermissionGrantedForContracts returns true for Owner, so contractHasValidPermission(V.perms, V) == true. (More generally, RecipientAddr can be set to any of V's signer addresses meeting threshold all public on-chain.) Accounts with no stored permissions have empty GetPermissions() and are immune. 4. Overwrite is unrestricted: UpdatePermission(V, attackerContract) (accounts.go:1863) replaces V's permission set with attacker-supplied signers; if the attacker supplies an Owner-type permission, no default is appended and V's prior control is fully evicted.

Code walkthrough

(a) The vulnerable handler — core/kapp/builtInFunctions/kleverUpdateAccountPermission.go: go func (e kleverUpdateAccountPermission) ProcessBuiltinFunction(vmInput vmcommon.ContractCallInput) (vmcommon.VMOutput, error) { ... address := vmInput.NextArg() // Arguments[0] — attacker-chosen target account V contract, err := e.getUpdateAccountPermissionContract(vmInput) // Arguments[1] — attacker-chosen new permissions ... acc, err := e.accountsCacher.LoadUser(address) // loads V ... // BUG: authorizes against vmInput.RecipientAddr (attacker-controlled), NOT vmInput.CallerAddr if !e.contractHasValidPermission(acc.GetPermissions(), vmInput.RecipientAddr) { // L91 return nil, errors.New("invalid permission operation") } // overwrites V's entire permission set with attacker-supplied signers resultCode, err := e.kappController.GetAccountsKApp().UpdatePermission(address, contract) ... }

(b) The check just name-matches recipientAddr against V's own signers — same file: go func (e kleverUpdateAccountPermission) contractHasValidPermission(permissions []state.Permission, recipientAddr []byte) bool { for , permission := range permissions { for , signer := range permission.Signers { if !bytes.Equal(signer.Address, recipientAddr) { // recipientAddr, not the authenticated caller continue } if signer.Weight >= permission.Threshold && permission.CheckPermissionGrantedForContracts(transaction.TXContractUpdateAccountPermissionContractType) { return true } } } return false }

(c) The dispatch makes RecipientAddr attacker-controlled — kvm/vmhost/vmhooks/baseOps.go:2464 prepareIndirectContractCallInput (invoked when a contract calls the built-in via ExecuteOnDestContext): go contractCallInput := &vmcommon.ContractCallInput{ VMInput: vmcommon.VMInput{ CallerAddr: sender, // the calling contract (authenticated) — NOT used by the handler Arguments: data, // attacker-chosen: [V, attackerPermissions] ... }, RecipientAddr: destination, // the contract's chosen dest argument — attacker sets this to V Function: string(function), }

(d) Every account with configured permissions is its own signer — core/kapp/accounts/accounts.go:1848 createDefaultOwnerPermission (appended by UpdatePermission when no Owner permission is supplied): go return &state.Permission{ Type: state.PermissionOwner, // Owner grants ALL contract types incl. type 22 Threshold: 1, Signers: []state.Key{ { Address: ownerAcc.AddressBytes(), Weight: 1 }, // the account signs for itself }, } So contractHasValidPermission(V.perms, RecipientAddr=V) finds V's own address as a Weight 1 >= Threshold 1 Owner signer → returns true.

(e) Contrast — the sibling handler does it correctly — core/kapp/builtInFunctions/kleverChangeOwnerAddress.go:86: go callerAddress := vmInput.CallerAddr // authenticated caller ... if !bytes.Equal(callerAddress, acc.GetOwnerAddress()) { // checks the CALLER, not RecipientAddr return nil, ErrOperationNotPermitted }

Putting it together — the attacker's contract call: ExecuteOnDestContext( gas, dest = V, // → RecipientAddr = V value = 0, function = "KleverUpdateAccountPermission", args = [ V, attackerOwnerPermsWithOnlyAttackerKey ], // Arguments[0]=V, Arguments[1]=new perms ) → CallerAddr = attackerContract (ignored), RecipientAddr = V, contractHasValidPermission(V.perms, V) == true → V's permissions overwritten with the attacker's key as sole Owner signer. The attacker never held a key of V and provided no signature from V.

POC

put the following poc testcase under /core/kapp/builtInFunctions/

POC Code: https://gist.github.com/mabdullah22/a41f90aa5ba86bbebf121f739bd5f5e9

Run: cd klever-go GOTOOLCHAIN=auto go test ./core/kapp/builtInFunctions/ -run TestPoCPermTakeover -v Output: TAKEOVER CONFIRMED: caller="attacker-contract" (attacker SC) rewrote account V="victim-account-V"; new sole owner signer="attacker-key-EVIL" --- PASS: TestPoCPermTakeover --- PASS: TestPoCPermTakeoverNoStoredPermsIsSafe The harm asserted is the takeover itself: after the call, V's permission set is a single Owner permission whose sole signer is the attacker's key; V's original owner signer is gone.

Impact

Full takeover of any account that has configured permissions i.e. every multisig / advanced-permission account , by an attacker who deploys a cheap smart contract and supplies only public on-chain addresses (no keys, no signatures from the victim). After takeover the attacker controls all of the victim's operations → theft or permanent lock of all the account's assets. Reachable via a permissionlessly-deployed contract (the plain-tx path is safe, so it is Critical-via-contract, not fully no-contract). No fork flag gates it.

Severity Critical: Impact High (full account/asset compromise)

Recommendation

Authorize against the authenticated caller, mirroring kleverChangeOwnerAddress: go if !e.contractHasValidPermission(acc.GetPermissions(), vmInput.CallerAddr) { ... } Reconcile the SC-call authority model: on the built-in path CallerAddr is the calling contract, so a contract should only be able to update permissions of accounts that legitimately list it as an authorized signer — never an arbitrary victim. Consider also requiring the target account (Arguments[0]) to equal the authorized caller's account, matching the native tx.GetSender() model.