code-423n4 / 2022-12-prepo-findings

0 stars 1 forks source link

Anyone can set the accountList object #243

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/packages/prepo-shared-contracts/contracts/AccountListCaller.sol#L10

Vulnerability details

Impact

The setAccountList function which is the function that is responsible to set the account list object is made public with no access control on the AccountListCaller contract,

Proof of Concept

truffle console --networkId 555
compile
attacker = "choose address from the ganache list"
payload = "IAccountList object"
AccountListCaller.deployed().then(function(instance){app=instance;})
app.setAccountList(IAccountList,{from:attacker})
// Transaction will succeed

Tools Used

Manual

Recommended Mitigation Steps

Add access control on the function

hansfriese commented 1 year ago

AccountListCaller is inherited by DepositHook, MintHook, and RedeemHook and setAccountList is an onlyRole function there.

Picodes commented 1 year ago

Invalid as @hansfriese explained. Maybe the contract could be made abstract and a comment could be added though

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid