DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: low
Valid

Lack of Router Address Validation and Approval Management in setRouter

Summary

The setRouter function lacks basic safeguards for router address validation and approval management. While protected by onlyManagement, implementing additional checks would follow security best practices and prevent potential misconfigurations.

Vulnerability Details

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

The function allows setting any address as router and does not revoke previous approvals, which deviates from security best practices for token approval management.

Impact

  1. If management mistakenly sets an incorrect router address, they would need to perform an additional transaction to correct it

  2. Previous router approvals remain active unnecessarily, slightly increasing the protocol's trust surface

Recommendation

Add basic safety checks and cleanup for router management:

function setRouter(address _router) external onlyManagement {
require(_router != address(0) && Address.isContract(_router), "Invalid router");
if(router != address(0)) {
underlying.safeApprove(router, 0);
}
router = _router;
underlying.safeApprove(router, type(uint256).max);
emit RouterUpdated(router);
}
Updates

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

Support

FAQs

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