Astaria contest - Deivitto's results

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898//src/PublicVault.sol#L548 https://github.com/code-423n4/2023-01-astaria/blob/40065677771348dbfde8c1ca442825ae37e2c3d0/src/AstariaVaultBase.sol#L50-L52 https://github.com/code-423n4/2023-01-astaria/blob/feb342a6666b9a97ef16d25151e020281acb1f5f/src/VaultImplementation.sol#L287

Vulnerability details

Summary

getLienEpoch() unsafe typecast leads to more time window for operations

Vulnerability Detail

Even though Solidity 0.8.x is used, type casts do not throw an error. A SafeCast library must be used everywhere a typecast is done. SafeCast Reference. Type cast with overflows doesn't throw an error / revert

Impact

Wrong values used over the code as overflow / underflow doesn't revert on cast

Proof of Concept

getLienEpoch is a public function used by handleBuyoutLien, updateVaultAfterLiquidation, _afterCommitToLien and this one being used internally by VaultImplementation#commitToLien() method.

If START() value is bigger than uint64, this would lead to the open windows being more time than expected opened, as the end - uint64(START() won't work as expected.

• PublicVault.sol#L548

uint256(Math.ceilDiv(end - uint64(START()), EPOCH_LENGTH()) - 1)

AstariaVaultBase

function START() public pure returns (uint256) {
    return _getArgUint256(61); //@audit returns uint256, therefore can overflow in uint(64)
} 

If value stored and get with _getArgUint256(61) is bigger than uint64, it will silently overflow, leading to wrong result

Code Snippet

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898//src/PublicVault.sol#L548

Tool used

Manual Review

Recommendation

Use safeCast library for conversions that can overflow / underflow