Algo Ssstablecoinsss
AI First Flight #2
Beginner FriendlyDeFi
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

[L-01] _redeem_collateral Does Not Validate Token Address Allowing Redemption of Unregistered Tokens

rensley

Root + Impact

Description

  • The _redeem_collateral() internal function does not validate that the token_collateral_address is a registered collateral token before attempting the redemption.

  • While _deposit_collateral() properly checks self.token_address_to_price_feed[token_collateral_address] != empty(address), the redemption function lacks this validation.

  • This inconsistency means the function relies solely on the user having a non-zero balance for that token, which could lead to unexpected behavior if an invalid token address is passed.

// Root cause in the codebase with @> marks to highlight the relevant section
​
@internal
def _deposit_collateral(
token_collateral_address: address, amount_collateral: uint256
):
assert amount_collateral > 0, "DSCEngine_NeedsMoreThanZero"
@> assert self.token_address_to_price_feed[token_collateral_address] != empty(
@> address
@> ), "DSCEngine__TokenNotAllowed" # Validation present here
# ...
​
@internal
def _redeem_collateral(
token_collateral_address: address,
amount_collateral: uint256,
_from: address,
_to: address,
):
@> # Missing: assert token is allowed
self.user_to_token_address_to_amount_deposited[_from][
token_collateral_address
] -= amount_collateral

Risk

Likelihood: Low

  • Reason 1 // Requires user to somehow have balance for unregistered token

  • Reason 2 // Current flow makes this unlikely since deposits are validated

Impact: Low

  • Impact 1 // Inconsistent validation between deposit and redeem

  • Impact 2 // Could cause confusion or unexpected reverts

  • Impact 3 // Defense-in-depth principle violated

Proof of Concept

The following demonstrates the inconsistency in validation between deposit and redeem functions. While the current flow protects against this since deposits are validated, the asymmetric validation creates potential for bugs if the code is modified in the future.

def test_redeem_validation_inconsistency():
random_token = deploy_collateral() # Unregistered token
with boa.env.prank(user):
# Deposit correctly rejects unregistered token
random_token.approve(dsce, AMOUNT)
with boa.reverts("DSCEngine__TokenNotAllowed"):
dsce.deposit_collateral(random_token, AMOUNT)
# But redeem has no such check
# If user somehow had balance (e.g., direct storage manipulation in test),
# redeem would attempt transfer without validating token is allowed
# This is inconsistent - both functions should validate token

Recommended Mitigation

Add the same token validation check to _redeem_collateral() for consistency and defense-in-depth. This ensures both deposit and redemption paths validate the token address, making the code more robust against future modifications.

@internal
def _redeem_collateral(
token_collateral_address: address,
amount_collateral: uint256,
_from: address,
_to: address,
):
+ assert self.token_address_to_price_feed[token_collateral_address] != empty(
+ address
+ ), "DSCEngine__TokenNotAllowed"
self.user_to_token_address_to_amount_deposited[_from][
token_collateral_address
] -= amount_collateral
log CollateralRedeemed(token_collateral_address, amount_collateral, _from, _to)
success: bool = extcall IERC20(token_collateral_address).transfer(
_to, amount_collateral
)
assert success, "DSCEngine_TransferFailed"
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!