Links

Trail of Bits 12/22/2021

Link to full audit PDF below:
Tokemak - Final Report [Trail of Bits 2021 12 22 - Tokemak].pdf
779KB
PDF
Link to audit PDF
Summary of Issues and Resolutions (Fix Log + Tokemak Notes):
  • Description: Access Controls - Manager’s logic contract
  • Auditor Severity Rating: High
  • Status: Fixed
  • Tokemak Assessment: Informational
  • Additional Notes: We suggested the reduction of this issue's rating to "Informational". Tokemak initialized all implementation contracts upon deployment and this exploit is not possible as described. With respect to roles, Rollover and Maintainer roles are not able to run arbitrary code, they can only run code that has been added by the administrator role which is backed by our multisig. In the event this contract might need to be deployed again at some point in the future we have implemented the fix suggested by Trail of Bits.
  • Description: Data Validation - Voting allocation duplicate
  • Auditor Severity Rating: High
  • Status: Fixed
  • Tokemak Assessment: High
  • Additional Notes: N/A
  • Description: Data Validation - Lack of incentive to vote early allows users to manipulate votes
  • Auditor Severity Rating: Medium
  • Status: Require Further Investigation
  • Tokemak Assessment: No Fix Required
  • Additional Notes: We suggested the reduction of this issue to "Informational." The issue appears to conflate two separate voting behaviors, 1) voting in the C.o.R.E. and, 2) voting as a Liquidity Director. In both cases the the issue is not applicable.
    Voting in C.o.R.E.:
    • Voting and rearranging of votes up until the last second is intended and expected. Any asset included in the event is welcomed to 'win' the event.
    Voting for Liquidity Direction (i.e. ongoing voting in live system):
    • Strong incentives exist to vote early. Early voting drives higher APRs for LPs to deposit into a reactor. Supporters of protocols are incentivized to signal early to attract the most LP deposits.
    • Alternatively, last minute voting to achieve higher voting APR is advantageous to the protocol rather than a risk. It provides further backstop to the less-collateralized reactors.
    Lastly, Liquidity Director voting relies on the idea that votes are public rather than private. Users must be aware of votes in the system to make an informed voting decision.
  • Description: Undefined Behavior - WithdrawalRequest event signature not implemented in Polygon contracts
  • Auditor Severity Rating: Medium
  • Status: Fixed
  • Tokemak Assessment: Medium
  • Additional Notes: N/A
  • Description: Staking's withheldLiquidity can become incorrect
  • Auditor Severity Rating: Medium
  • Status: Not Fixed
  • Tokemak Assessment: Fixed
  • Additional Notes: ToB Report indicates 'Not Fixed' however this variable was removed, as it was not needed.
  • Description: Data Validation - updateUserVoteTotals only decreases voting power
  • Auditor Severity Rating: Medium
  • Status: Not Fixed
  • Tokemak Assessment: No Fix Required
  • Additional Notes: ToB report indicates 'Not Fixed' however Tokemak suggests the reduction of this issue to "Informational," without a fix required. updateUserVoteTotals is intentionally meant to only decrease. A user's votes determine where they are putting their funds at risk. Should we increase their votes upon a balance increase without their input, we would effectively put their additional funds at risk without their input.
  • Description: Data Validation - Lack of return value checks can lead operations to silently fails
  • Auditor Severity Rating: Low
  • Status: Fixed
  • Tokemak Assessment: Low
  • Additional Notes: N/A
  • Description: Access Controls - Transfers are allowed on EthPool when the pool is paused
  • Auditor Severity Rating: Low
  • Status: Fixed
  • Tokemak Assessment: Low
  • Additional Notes: N/A
  • Description: Data Validation - Divide-before-multiply can result in incorrect votedAmt
  • Auditor Severity Rating: Low
  • Status: Fixed
  • Tokemak Assessment: Low
  • Additional Notes: N/A
  • Description: Data Validation - Lack of input validation on the to parameter can result in a loss of funds
  • Auditor Severity Rating: Informational
  • Status: Fixed
  • Tokemak Assessment: Informational
  • Additional Notes: N/A
  • Description: Auditing and Logging - Insufficient event generation
  • Auditor Severity Rating: Informational
  • Status: Fixed
  • Tokemak Assessment: Low
  • Additional Notes: N/A
  • Description: Undefined Behavior - executeRollover deviates from the specification
  • Auditor Severity Rating: Informational
  • Status: Fixed
  • Tokemak Assessment: Informational
  • Additional Notes: N/A
  • Description: Undefined Behavior - Divergence between the EthPool and Pool contracts and the Staking contract
  • Auditor Severity Rating: Informational
  • Status: Not Fixed
  • Tokemak Assessment: Fixed
  • Additional Notes: Updated documentation.
  • Description: Undefined Behavior - Removing allowed functions does not remove delegations that are already active on them
  • Auditor Severity Rating: Undetermined
  • Status: Fixed
  • Tokemak Assessment: Undetermined
  • Additional Notes: Removed ability to remove functions.
  • Description: Undefined Behavior - Inconsistent use of 0x protocol specification
  • Auditor Severity Rating: Informational
  • Status: Not Fixed
  • Tokemak Assessment: Fixed
  • Additional Notes: Updated documentation.
  • Description: Denial of Service - Inefficient slashing mechanism
  • Auditor Severity Rating: Informational
  • Status: Not Fixed
  • Tokemak Assessment: Fixed
  • Additional Notes: Function was updated to take an array of addresses and amounts.
  • Description: Patching - Unused inheritance is error-prone
  • Auditor Severity Rating: Informational
  • Status: Fixed
  • Tokemak Assessment: Informational
  • Additional Notes: Implemented pausable.
  • Description: Data Validation - Manager’s getters may cause out-of-gas issues
  • Auditor Severity Rating: Low
  • Status: Not Fixed
  • Tokemak Assessment: Low
  • Additional Notes: The limit on read operations is sufficient to support our requirements, so we elected to maintain a simpler implementation.
  • Description: Undefined Behavior - Unclear incentives for TOKE stakers
  • Auditor Severity Rating: Undetermined
  • Status: Require Further Investigation
  • Tokemak Assessment: Remove
  • Additional Notes: Note, the outlined issue does not cause it to become unsustainable. In both described exploit scenarios, the actions are detrimental to the attacker by receiving a lower overall APR while not decreasing risk.
  • Description: Timing - Risk of market manipulation via front-running of managers’ actions
  • Auditor Severity Rating: High
  • Status: Require Further Investigation
  • Tokemak Assessment: Out of Scope (Managed Off-chain)
  • Additional Notes: Issue is a copy/paste of initial general security review and was addressed in system design and implementation. We acknowledge the attack vector and it is addressed by off-chain components of the system which were out of scope of the smart contracts reviewed by the audit.
  • Description: Timing - Risk of market manipulation in bailout processes
  • Auditor Severity Rating: High
  • Status: Require Further Investigation
  • Tokemak Assessment: Out of Scope (Managed Off-chain)
  • Additional Notes: Issue is a copy/paste of initial general security review and was addressed in system design and implementation. We acknowledge the attack vector and it is addressed by off-chain components of the system which were out of scope of the smart contracts reviewed by the audit.
  • Description: Timing - Risk of token slashing caused by market manipulation
  • Auditor Severity Rating: High
  • Status: Require Further Investigation
  • Tokemak Assessment: Out of Scope (Managed Off-chain)
  • Additional Notes: Issue is a copy/paste of initial general security review and was addressed in system design and implementation. We acknowledge the attack vector and it is addressed by off-chain components of the system which were out of scope of the smart contracts reviewed by the audit.