The Owner role has essentially complete access to all contract functionality. This causes a single point of failure should the Owner role ever act in bad faith.
The Owner can set and unset (including setting themselves as) the following roles:
Blocklister
MasterMinter
Pauser
Rescuer
This gives the Owner complete access to minting and freezing of core contract functionality. Consider separating the responsibilities of the owner into one or more new roles to reduce the risk should the Owner role become compromised. A multisig for the Owner could also help reduce the risk of an Owner role compromise.
The approve function does not validate that the previous allowance was already used up before updating the allowance. If a user uses the approve function instead of decreaseAllowance to lower their allowance, they would be subject to a double withdrawal attack.
Scenario:
If User A has an allowance of 10 JPYC for User B and they use the approve(address spender, uint256 value) function to change the amount to 5 JPYC, user B could frontrun that approve transaction to perform a transferFrom(address from, address to, uint256 value) while the allowance is still 10, setting the allowance to 0. Now when user A’s transaction is processed, this reupdates the allowance to 5. User B could now call the transferFrom(address from, address to, uint256 value) function once more for an additional 5 JPYC bringing the the total JPYC transferred to user B to 15.
JPYC Contest
February 25, 2022
@securerodd #
Findings
Low Risk Findings
1. Centralization Concerns
The
Ownerrole has essentially complete access to all contract functionality. This causes a single point of failure should theOwnerrole ever act in bad faith.The
Ownercan set and unset (including setting themselves as) the following roles:BlocklisterMasterMinterPauserRescuerThis gives the
Ownercomplete access to minting and freezing of core contract functionality. Consider separating the responsibilities of the owner into one or more new roles to reduce the risk should theOwnerrole become compromised. A multisig for theOwnercould also help reduce the risk of anOwnerrole compromise.2. Double Withdrawal Attack (approve + transferFrom)
The approve function does not validate that the previous allowance was already used up before updating the allowance. If a user uses the approve function instead of decreaseAllowance to lower their allowance, they would be subject to a double withdrawal attack.
Scenario: If User A has an allowance of 10 JPYC for User B and they use the
approve(address spender, uint256 value)function to change the amount to 5 JPYC, user B could frontrun that approve transaction to perform atransferFrom(address from, address to, uint256 value)while the allowance is still 10, setting the allowance to 0. Now when user A’s transaction is processed, this reupdates the allowance to 5. User B could now call thetransferFrom(address from, address to, uint256 value)function once more for an additional 5 JPYC bringing the the total JPYC transferred to user B to 15.More information about this documented issue and potential mitigations are outlined in this doc: https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit