hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd

Liquid staking protocol for Ethereum
Other
0 stars 0 forks source link

OsToken::burnShares() - L146: no check to see if `balanceOf[owner] >= shares`. Would revert here anyway due to underflow error, but maybe protocol would have wanted to then set `shares` = balanceOf[owner], and then deduct all that instead... like some protocols do...? #137

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @dappconsulting Submission hash (on-chain): 0xa01ed2f9efb22c5cc229db911375bdc18accea086d8d98d90112fc41aef12566 Severity: low

Description: Description\

OsToken::burnShares() - L146: no check to see if balanceOf[owner] >= shares. Would revert here anyway due to underflow error, but maybe protocol would have wanted to then set shares = balanceOf[owner], and then deduct all that instead... like some protocols do...?

https://github.com/stakewise/v3-core/blob/9c30c45878397aa97918cbafcc6a62e4be4bbd4d/contracts/osToken/OsToken.sol#L146

It could also be that the vault calling this function would ensure that balanceOf[owner] >= shares, I have not checked this.

Attack Scenario\ None.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

tsudmi commented 1 year ago

Similarly, we're fine with the underflow revert here, we don't want to set shares = balanceOf[owner] as it means burning less shares than user requested.