Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function redeemParams should return default value #189

Closed
code423n4 opened this issue May 22, 2023 · 2 comments
Closed

Function redeemParams should return default value #189

code423n4 opened this issue May 22, 2023 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-79 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

code423n4 commented May 22, 2023

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L235-#L239

Vulnerability details

Impact

  • If JBXBuybackDelegate is used as data source for redeem, users could not redeem their tokens.

Proof of Concept

The contract JBXBuybackDelegate is a data source and according to the doc: A data source contract can be used to provide custom data to the JBPayoutRedemptionPaymentTerminal3_1.pay(...) transaction and/or the JBPayoutRedemptionPaymentTerminal3_1.redeemTokensOf(...) transaction.

Since JBXBuybackDelegate implements interface IJBFundingCycleDataSource, it must override function redeemParams and the contract decides to leave it empty:

function redeemParams(JBRedeemParamsData calldata _data)
        external
        override
        returns (uint256 reclaimAmount, string memory memo, JBRedemptionDelegateAllocation[] memory delegateAllocations)
    {}

Because the function is empty, if JBXBuybackDelegate is used as datasource for redeem, users would not be able to redeem their tokens, since the returned reclaimAmount from redeemParams is always 0.

Although this contract JBXBuybackDelegate is meant to be used for pay only and not for redeem, the contract should also let function redeemParams return default value (to not interfere with redeem process) so that if a user use JBXBuybackDelegate as datasource for redeem (it's perfectly possible since this contract implements IJBFundingCycleDataSource interface), the contract would not make reclaimedAmount = 0, as recommended in the doc: https://docs.juicebox.money/dev/build/treasury-extensions/data-source/#examples

// This is unused but needs to be included to fulfill IJBFundingCycleDataSource.
  function redeemParams(JBRedeemParamsData calldata _data)
    external
    pure
    override
    returns (
      uint256 reclaimAmount,
      string memory memo,
      IJBRedemptionDelegate delegate
    )
  {
    // Return the default values.
    return (_data.reclaimAmount.value, _data.memo, IJBRedemptionDelegate(address(0)));
  }

Tools Used

Recommended Mitigation Steps

I recommend returning default values from redeemParams function as the code above.

Assessed type

Library

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels May 22, 2023
code423n4 added a commit that referenced this issue May 22, 2023
@c4-pre-sort
Copy link

dmvt marked the issue as duplicate of #79

@c4-judge
Copy link

c4-judge commented Jun 2, 2023

dmvt marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-79 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants