code-423n4 / 2023-10-brahma-findings

8 stars 7 forks source link

Account should be able to add max time it would want its transaction to be executed. #431

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/ExecutorPlugin.sol#L68-L77

Vulnerability details

Impact

Account is not allowed to specify the expiration time for its request execution, since most request run arbitrary calls, which might be time bound.

Proof of Concept

Alot of transaction or calls on the blockchain are time bound, lets say for example Alice wants to allow the sub account to participate in a time bound auction, with the current round expiring in the next ten minutes before another round starts which would be more expensive than the previous, Alice signs a request execution and the request remains in the mempool (for those chains with a public mempool), for more time than the current round, and when miners finally execute the transaction it falls into another round, which might cost Alice more that bargained.

Tools Used

Manual

Recommended Mitigation Steps

There should be an option to specify request execution expiration.

Assessed type

Timing

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

raymondfam commented 1 year ago

OOS. Inadequate elaboration.

alex-ppg commented 1 year ago

In order for an executor's transaction to execute, multiple steps need to take place including a validation by the relevant policy commitment. The policy commitment can, in fact, be represented by a smart contract that validates the policies and can enforce an expiry on executor transactions if needed.

As @raymondfam has correctly stated, the exhibit lacks sufficient proof that the absence of an expiry can materialize into a vulnerability.

c4-judge commented 1 year ago

alex-ppg marked the issue as unsatisfactory: Insufficient proof