code-423n4 / 2024-01-renft-findings

2 stars 0 forks source link

Rental safes are unable to interact with other contracts via the receive ether function #541

Open c4-bot-7 opened 10 months ago

c4-bot-7 commented 10 months ago

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L329

Vulnerability details

Impact

Rental safes cannot call the receive ether function of other contracts because a function selector must be provided.

Proof of Concept

Guard.checkTransaction prevents any safe execution where data.length < 4:

if (data.length < 4) {
    revert Errors.GuardPolicy_FunctionSelectorRequired();
}

This means that calldata must be provided. However, there is a function which only executes if no calldata is provided. "The receive function is executed on a call to the contract with empty calldata."

There is no reason to prevent execution of receive functions, and the inability to do so causes a DoS of rental safe users under reasonable usage.

Tools Used

Recommended Mitigation Steps

There is no need to enforce that calldata is provided as it's impossible to call any guarded functions without calldata anyway. Simply remove the following statement from Guard.checkTransaction:

if (data.length < 4) {
    revert Errors.GuardPolicy_FunctionSelectorRequired();
}

Assessed type

DoS

c4-pre-sort commented 10 months ago

141345 marked the issue as sufficient quality report

141345 commented 10 months ago

better with receive() function.

feature improvement QA seems more appropriate

c4-sponsor commented 10 months ago

Alec1017 (sponsor) confirmed

c4-sponsor commented 10 months ago

Alec1017 marked the issue as disagree with severity

Alec1017 commented 10 months ago

Agree that QA seems appropriate

c4-judge commented 10 months ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

0xean marked the issue as grade-c

c4-judge commented 10 months ago

0xean marked the issue as grade-b