stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Accidental `renounceOwnership()` call can disrupt key operations in multiple contracts.

The Seraphs

Title

Accidental renounceOwnership() call can disrupt key operations in multiple contracts.

Severity

Medium

Relevant GitHub Links

  1. https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/RESDLTokenBridge.sol#L16C1-L16C39

  2. https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/SDLPoolCCIPControllerPrimary.sol#L11C1-L11C65

  3. https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/SDLPoolCCIPControllerSecondary.sol#L14C1-L14C67

  4. https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/WrappedTokenBridge.sol#L19C1-L19C55

  5. https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/LinearBoostController.sol#L10C1-L10C44

Summary

Ownable contains a function named renounceOwnership() which can be used to remove the ownership of contracts in a protocol.

This can lead to SDLPoolCCIPControllerPrimary, SDLPoolCCIPControllerPrimary, WrappedTokenBridge, LinearBoostController and RESDLTokenBridge contracts becoming disowned, which will then break critical functions of the protocol.

Vulnerability Details

The WrappedTokenBridge, LinearBoostController and RESDLTokenBridge contracts inherit from Ownable, SDLPoolCCIPControllerPrimary from SDLPoolCCIPController which inherits Ownable, and SDLPoolCCIPControllerSecondary inherits from SDLPoolCCIPControllerPrimary; and hence inherit renounceOwnership() function.

The owner could accidentally (or intentionally) call renounceOwnership() which transfers ownership to address(0). This will break numerous functions within each contract referenced that has the onlyOwner() modifier assigned. Below are a list of those functions:

SDLPoolCCIPControllerPrimary

  • setRewardsInitiator()

  • setWrappedRewardToken()

  • approveRewardTokens()

  • removeWhitelistedChain()

  • addWhitelistedChain()

SDLPoolCCIPControllerSecondary

  • setExtraArgs()

WrappedTokenBridge

  • recoverTokens()

  • transferTokens()

LinearBoostController

  • setMaxLockingDuration()

  • setMaxBoost()

RESDLTokenBridge.

  • setExtraArgs()

POC

Add this test to test/core/ccip/sdl-pool-ccip-controller-primary.test.ts

it.only('renounce ownership', async () => {
console.log("Owner before", await controller.owner())
// set max link fee
await controller.setMaxLINKFee(toEther(100))
// console out the max link fee
console.log("Set max link fee with onlyOwner modifier", await controller.maxLINKFee())
// renounce ownership using renounceOwnership() from owner contract
await expect(controller.renounceOwnership())
// set max link fee and expect revert
await expect(controller.setMaxLINKFee(toEther(200))).to.be.revertedWith('Ownable: caller is not the owner')
// console out the max link fee
console.log("set max link fee hasn't changed", await controller.maxLINKFee())
// console out the owner
console.log("Owner after", await controller.owner())
})

Tools Used

Manual Review

Recommendations

Disable renounceOwnership() if function in the Ownable contract not required.

+ function renounceOwnership() public override onlyOwner {
+ revert ("Not allowed");
+ }
Updates

Lead Judging Commences

0kage Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

renounce

accidentally renouncing ownership

pxng0lin Submitter
almost 2 years ago
pxng0lin Submitter
almost 2 years ago
0kage Lead Judge
almost 2 years ago
pxng0lin Submitter
almost 2 years ago
0kage Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

renounce

accidentally renouncing ownership

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.