Summary & Vulnerability Details
updateYield() calculates the yield as zethTotalNew - zethTotal. If zethTotalNew is less then zethTotal, then no yield is accrued.
File: contracts/libraries/LibVault.sol
55
56 * @notice Updates the vault yield rate from staking rewards earned by bridge contracts holding LSD
57 * @dev Does not distribute yield to any individual owner of shortRecords
58 *
59 * @param vault The vault that will be impacted
60 */
61
62 function updateYield(uint256 vault) internal {
63 AppStorage storage s = appStorage();
64
65 STypes.Vault storage Vault = s.vault[vault];
66 STypes.VaultUser storage TAPP = s.vaultUser[vault][address(this)];
67
68 @> uint88 zethTotalNew = uint88(getZethTotal(vault));
69 uint88 zethTotal = Vault.zethTotal;
70 uint88 zethCollateral = Vault.zethCollateral;
71 uint88 zethTreasury = TAPP.ethEscrowed;
72
73
74 if (zethTotalNew <= zethTotal) return;
75 uint88 yield = zethTotalNew - zethTotal;
76 Vault.zethTotal = zethTotalNew;
...
...
...
On L68, getZethTotal() returns a uint256 which is down-casted to uint88 in an unsafe way. Imagine this:
zethTotal in the vault is equal to type(uint88).max
After a few moments, when updateYield() is called, it internally calls getZethTotal() to get a zETH balance which has crossed over to a value of type(uint88).max + 100
Since L68 typecasts it to uint88, zethTotalNew will appear just as 99.
Function will return with no accrued yield on L74.
Function will never update the new Vault.zethTotal on L76 and hence this will keep on happening again & again each time updateYield is called.
PoC
Create a new file under test/ folder named MathCastingUpdateYield.t.sol and run the following code via forge test --mt test_casting_updateYield -vv:
pragma solidity 0.8.21;
contract MathCastingUpdateYield {
uint88 private zethTotal;
function setUp() public {
zethTotal = type(uint88).max;
}
function mockGetZethTotal() public pure returns (uint256) {
return 309485009821345068724781055 + 100;
}
function test_casting_updateYield() public view {
require(uint88(mockGetZethTotal()) > zethTotal, "Downcasting issue");
}
}
Output:
Running 1 test for test/MathCastingUpdateYield.t.sol:MathCastingUpdateYield
[FAIL. Reason: Downcasting issue] test_casting_updateYield() (gas: 2628)
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 466.81µs
Impact
updateYield() breaks permanently due to high zETH in the vault, but protocol continues to operate without any reverts. Difficult to take any remedial action with no warning system.
Tools Used
Manual inspection.
Recommendations
Use a safe typecasting library or a custom function which reverts in such cases. Example of such a function could be:
function safeU88(uint256 n) internal pure returns (uint88) {
if (n > type(uint88).max) revert Errors.InvalidUInt88();
return uint88(n);
}
And then use it in code:
File: contracts/libraries/LibVault.sol
55 /**
56 * @notice Updates the vault yield rate from staking rewards earned by bridge contracts holding LSD
57 * @dev Does not distribute yield to any individual owner of shortRecords
58 *
59 * @param vault The vault that will be impacted
60 */
61
62 function updateYield(uint256 vault) internal {
63 AppStorage storage s = appStorage();
64
65 STypes.Vault storage Vault = s.vault[vault];
66 STypes.VaultUser storage TAPP = s.vaultUser[vault][address(this)];
67 // Retrieve vault variables
- 68 uint88 zethTotalNew = uint88(getZethTotal(vault)); // @dev(safe-cast)
+ 68 uint88 zethTotalNew = safeU88(getZethTotal(vault)); // @audit : safe-casted now
69 uint88 zethTotal = Vault.zethTotal;
70 uint88 zethCollateral = Vault.zethCollateral;
71 uint88 zethTreasury = TAPP.ethEscrowed;
72
73 // Calculate vault yield and overwrite previous total
74 if (zethTotalNew <= zethTotal) return;
75 uint88 yield = zethTotalNew - zethTotal;
76 Vault.zethTotal = zethTotalNew;
