yAxis contest - cmichel's results

Handle

cmichel

Vulnerability details

The acceptGovernance function in Alchemist & Transmuter does not reset pendingGovernance to zero.

Impact

The pending governor can repeatedly accept the governance, emitting a GovernanceUpdated event each time, bloating listeners for this event with unnecessary data.

Recommended Mitigation Steps

Validate the parameters.

Handle

cmichel

Vulnerability details

The updateAccount function should capture the latest distributed yield to the Transmuter (stored in buffer) and therefore work with the latest totalDividendPoints variable. This variable is updated when running a phase distribution with runPhasedDistribution.

Unlike all other function that call updateAccount, the unstake function does not first run a runPhasedDistribution modifer to distribute the latest yield.

Impact

Users that unstake lose out on some yield by not having their alTokens transmuted.

Recommended Mitigation Steps

Call runPhasedDistribution in unstake before the updateAccount call, as in stake or transmute.

Handle

cmichel

Vulnerability details

The Alchemist.migrate function pushes the adapter to the _vaults list. It does not check if the adapter is already registered.

Impact

Duplicate adapters can be registered and will then track the totalDeposited independently.

Recommended Mitigation Steps

Check if adapter already exists in _vaults before adding it.

Handle

cmichel

Vulnerability details

Some tokens (like USDT) don't correctly implement the EIP20 standard and their approve function returns void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert.

For the tokens that return a success value, the contract does not check it.

Non-safe transfers are used in:

• Alchemist._distributeToTransmuter: token.approve(transmuter, amount);

Impact

Tokens that return false on a failed approve or that don't correctly implement the latest EIP20 spec, like USDT, will be unusable in the protocol as they revert the transaction because of the missing return value.

Recommended Mitigation Steps

We recommend using OpenZeppelin’s SafeERC20 versions with the safeApprove function that handle the return value check as well as non-standard-compliant tokens.

Handle

cmichel

Vulnerability details

The difference between the Transmuter.transmute function and calling forceTransmute(self) on the own account is that transmute reallocates any excess yield to all other stakers, while forceTransmute pays out the excess yield to the user.

"One interesting side effect of this process is that a user's position can be over-filled. For example, a user deposits 1000 alUSD, and some time later, that staking position will have 1050 DAI filled. Once a user has gone past this limit, it is possible for other users to initialize a claim on their behalf. For any balance of DAI over their staked alUSD, the user who claimed on their behalf will have that surplus in amount immediately convert their staked alUSD." (forceTransmute) vs " If the person who staked the alUSD claims their DAI with a surplus, the surplus will be cycled back into the Transmuter and be spread globally." (transmute) https://alchemix-finance.gitbook.io/alchemix-finance/transmuter

Impact

There doesn't seem to be an economic incentive for rational agents to call transmute instead of just force-transmuting themselves.

Recommended Mitigation Steps

Remove the transmute function or remove forceTransmute and make its behavior the default for transmute.