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

[VEN-1798]: Fix Certik 2023-08 findings for PSR/RiskFund/Shortfall contracts #287

Merged
merged 8 commits into from
Aug 16, 2023

Commits on Aug 8, 2023

  1. fix: [SSV-01] use _transferOutOrTrackDebt upon closing the auction

    Problem: The code used safeTransfer when closing an auction, which
    may fail due to token restrictions. Failure to close an auction is
    a DoS vector.
    
    Solution: If transferring out is not possible, track debt and allow
    the auction winner to withdraw funds later.
    kkirka committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    5ca628c View commit details
    Browse the repository at this point in the history

Commits on Aug 9, 2023

  1. fix: [VPB-04] fix potential re-entrancy issues

    Problem: There's a potential for cross-function reentrancy in
    RiskFund, ProtocolShareReserve, Shortfall contracts arising from
    the fact that nonReentrant modifier is not present in all external
    functions open to anyone.
    
    Solution: Use nonReentrant where applicable. Follow checks-effects-
    -interactions when emitting events.
    kkirka committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    42a9f78 View commit details
    Browse the repository at this point in the history
  2. refactor!: [RHR-01] make poolsAssetsReserves internal

    Problem: We have a view function getPoolAssetReserve that checks
    whether the supplied arguments are valid. Compiler-generated
    function duplicates the functionality and adds some gas overhead
    to function dispatching.
    
    Solution: Make poolsAssetsReserves internal so that we don't have
    a compiler-generated version anymore.
    kkirka committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    9085787 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    5effe7b View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    1dbf136 View commit details
    Browse the repository at this point in the history

Commits on Aug 14, 2023

  1. fix: [RFR-01][RFR-03] unify the meaning of minAmountToConvert

    Problem: With the evolution of the code, the meaning of the
    conversion threshold started to change. As a result, we ended up
    comparing the amount out (in tokens), balance of underlying *
    oracle price (in dollars), with minAmountToConvert. While in most
    cases these values would be close to each other as long as some
    stablecoin is used for the threshold, it might be problematic
    to set this threshold correctly, considering the potential slippage.
    
    Solution: Expect amountOutMin * base price to be greater than
    minAmountToConvert. Adjust the failing test accordingly.
    kkirka committed Aug 14, 2023
    Configuration menu
    Copy the full SHA
    db0e468 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    e4820ff View commit details
    Browse the repository at this point in the history
  3. fix: revert on approval failures

    Problem: OpenZeppelin safeApprove implementation is deprecated, so
    its usage was removed from the RiskFund contract in favor of plain
    approve call. However, this is not the correct way to fix the issue
    since the updated version of the code doesn't handle the case when
    tokens return false to indicate an error.
    
    Solution: Use a custom library similar to Uniswap's TransferHelpers
    to make an approve call supporting:
      * false to indicate failure
      * revert to indicate failure
      * non-compliant tokens that don't return any value
    kkirka committed Aug 14, 2023
    Configuration menu
    Copy the full SHA
    6c559f1 View commit details
    Browse the repository at this point in the history