Unauthorized coins transfer from locking account(s)
Critical
C
Cosmos
Submitted None
Actions:
Reported by
unknown_feature
Vulnerability Details
Technical details and impact analysis
### Summary of Impact
An attacker can transfer money from the locking account they don't own(if the account has unlocked funds, can be after locking period is over). The POC is done for `periodic-locking-account`. But it seems like more locking accounts are affected because the issue seems to be in [SendCoins](https://github.com/cosmos/cosmos-sdk/blob/7e391959b9aebf055294b24b7f392346709dae64/x/accounts/defaults/lockup/lockup.go#L285-L284). And it's used by all of them. When it calls the function it passes sender from the message `msg.Sender` in [checkSender](https://github.com/cosmos/cosmos-sdk/blob/7e391959b9aebf055294b24b7f392346709dae64/x/accounts/defaults/lockup/lockup.go#L333) it checks that the sender == owner. But the issue here is that msg.Sender is not validated anywhere bc this message is packed into [MsgExecute](https://github.com/cosmos/cosmos-sdk/blob/7e391959b9aebf055294b24b7f392346709dae64/api/cosmos/accounts/v1/tx.pulsar.go#L4444-L4443). While executing it sets original sender into the [context ](https://github.com/cosmos/cosmos-sdk/blob/7e391959b9aebf055294b24b7f392346709dae64/x/accounts/keeper.go#L284). And in case of multisig account it correctly takes it from [ctx](https://github.com/cosmos/cosmos-sdk/blob/7e391959b9aebf055294b24b7f392346709dae64/x/accounts/defaults/multisig/account.go#L168) . At least in those places I looked at. But these locking accounts take it from the message. And it can be anything.
#### POC scenario
1. We first create a periodic-locking-account at the [victim ](https://gist.github.com/unknownfeature/d0b8cfcf263904d9be4707dded38c706#file-main-rs-L28). Locking period is small so we wouldn't wait long.
2. Then we wait for locking period to end and transfer money to the [attacker](https://gist.github.com/unknownfeature/d0b8cfcf263904d9be4707dded38c706#file-main-rs-L55)
### Steps to Reproduce
Go and Rust required
1. Checkout latest version of [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) and run `make build`. Note the path to binariy and when it's done replace the path [in setup_chain](https://gist.github.com/unknownfeature/d0b8cfcf263904d9be4707dded38c706#file-setup_chain-L18).
2. Create rust project with attached `Cargo.toml`. Download all attached *.rs files and put them into `src` folder.
3. Make sure setup_chain has execute permission. Run `./setup_chain`. Wait for it to start. And then run the rust project. See the POC video.
{F4027705}
### Workarounds
Doesn't seem like there are any
### Supporting Material/References
1. setup_chain - script that sets up and starts the chain
2. main.rs, types.rs, client.rs, func.rs, msg.rs and Cargo.toml the attack itself. Sorry there are a lot of files. I'll probably push it all somewhere and update the report.
3. attack.mov - the video of POC
## Impact
An attacker can take over someone's funds that were locked and then unlocked on the locking account. The POC particularly demonstrates `periodic-locking-account`. But there are reasons to believe more locking accounts are affected.
Report Details
Additional information and metadata
State
Closed
Substate
Resolved
Submitted
Weakness
Improper Access Control - Generic