Trail of Bits 12/22/2021
Link to full audit PDF below:
Summary of Issues and Resolutions (Fix Log + Tokemak Notes):
TOB-TOKE-001: Access Controls | Status: Fixed
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.
TOB-TOKE-002: Data Validation | Status: Fixed
Description: Data Validation - Voting allocation duplicate
Auditor Severity Rating: High
Status: Fixed
Tokemak Assessment: High
Additional Notes: N/A
TOB-TOKE-003: Data Validation | Status: Require Further Investigation (Expand for Details)
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.
TOB-TOKE-004: Undefined Behavior | Status: Fixed
Description: Undefined Behavior -
WithdrawalRequest
event signature not implemented in Polygon contractsAuditor Severity Rating: Medium
Status: Fixed
Tokemak Assessment: Medium
Additional Notes: N/A
TOB-TOKE-005: Undefined Behavior | Status: Not Fixed (Expand for Details)
Description: Staking's
withheldLiquidity
can become incorrectAuditor 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.
TOB-TOKE-006: Data Validation | Status: Not Fixed (Expand for Details)
Description: Data Validation -
updateUserVoteTotals
only decreases voting powerAuditor 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.
TOB-TOKE-007: Data Validation | Status: Fixed
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
TOB-TOKE-008: Access Controls | Status: Fixed
Description: Access Controls - Transfers are allowed on
EthPool
when the pool is pausedAuditor Severity Rating: Low
Status: Fixed
Tokemak Assessment: Low
Additional Notes: N/A
TOB-TOKE-009: Data Validation | Status: Fixed
Description: Data Validation - Divide-before-multiply can result in incorrect
votedAmt
Auditor Severity Rating: Low
Status: Fixed
Tokemak Assessment: Low
Additional Notes: N/A
TOB-TOKE-010: Data Validation | Status: Fixed
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
TOB-TOKE-011: Auditing and Logging | Status: Fixed
Description: Auditing and Logging - Insufficient event generation
Auditor Severity Rating: Informational
Status: Fixed
Tokemak Assessment: Low
Additional Notes: N/A
TOB-TOKE-012: Undefined Behavior | Status: Fixed
Description: Undefined Behavior -
executeRollover
deviates from the specificationAuditor Severity Rating: Informational
Status: Fixed
Tokemak Assessment: Informational
Additional Notes: N/A
TOB-TOKE-013: Undefined Behavior | Status: Not Fixed (Expand for Details)
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.
TOB-TOKE-014: Undefined Behavior | Status: Fixed
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.
TOB-TOKE-015: Undefined Behavior | Status: Not Fixed (Expand for Details)
Description: Undefined Behavior - Inconsistent use of 0x protocol specification
Auditor Severity Rating: Informational
Status: Not Fixed
Tokemak Assessment: Fixed
Additional Notes: Updated documentation.
TOB-TOKE-016: Denial of Service | Status: Not Fixed (Expand for Details)
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.
TOB-TOKE-017: Patching | Status: Fixed
Description: Patching - Unused inheritance is error-prone
Auditor Severity Rating: Informational
Status: Fixed
Tokemak Assessment: Informational
Additional Notes: Implemented pausable.
TOB-TOKE-018: Data Validation | Status: Not Fixed (Expand for Details)
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.
TOB-TOKE-019: Undefined Behavior | Status: Require Further Investigation (Expand for Details)
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.
TOB-TOKE-020: Timing | Status: Require Further Investigation (Expand for Details)
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.
TOB-TOKE-021: Timing | Status: Require Further Investigation (Expand for Details)
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.
TOB-TOKE-022: Timing | Status: Require Further Investigation (Expand for Details)
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.
Last updated