Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe type casting #195

Open
code423n4 opened this issue Nov 15, 2021 · 3 comments
Open

Unsafe type casting #195

code423n4 opened this issue Nov 15, 2021 · 3 comments
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working LinearVesting sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-11-vader/blob/429970427b4dc65e37808d7116b9de27e395ce0c/contracts/tokens/vesting/LinearVesting.sol#L137-L138

vester.amount -= uint192(vestedAmount);

Downcasting from uint256/int256 in Solidity does not revert on overflow. This can easily result in undesired exploitation or bugs.

Recommendation

Consider using SafeCast library from OpenZeppelin.

https://docs.openzeppelin.com/contracts/4.x/api/utils#SafeCast

@code423n4 code423n4 added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working labels Nov 15, 2021
code423n4 added a commit that referenced this issue Nov 15, 2021
@SamSteinGG SamSteinGG added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 25, 2021
@SamSteinGG
Copy link
Collaborator

type casting is safe as vestedAmount will at all times be lses than vester.amount which in turn fits within the uint192 limit.

@alcueca
Copy link
Collaborator

alcueca commented Dec 11, 2021

Not necessarily, _getClaim can increase the amount parameter that is passed in, and return a value higher than uint192.

https://github.com/code-423n4/2021-11-vader/blob/429970427b4dc65e37808d7116b9de27e395ce0c/contracts/tokens/vesting/LinearVesting.sol#L294

@SamSteinGG
Copy link
Collaborator

@alcueca The value is guaranteed to be less than the amount that is being vested, otherwise the logical constraints of vesting are broken. Can you elaborate how it can exceed the uint limit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working LinearVesting sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

4 participants