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

Outdated Solidity Version Provides No Protections Against Arithmetic Underflows And Overflows #185

Open
code423n4 opened this issue Nov 30, 2021 · 2 comments
Labels
0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

leastwood

Vulnerability details

Impact

Malt Finance uses solidity version >=0.6.6 throughout all of its contracts. This solidity version provides no protections against arithmetic underflows and overflows. As a result, it is incredibly difficult to guarantee that the protocol enforces the necessary arithmetic checks during sensitive actions.

There are several instances where the OpenZeppelin's SafeMath library is not used. This exposes the protocol to potential exploits via arithmetic underflows and overflows. The liveness of the protocol depends on safety guarantees that are not provided/enforced. Therefore, this issue should be deemed high severity.

Proof of Concept

Solidity version shown in all contracts.

Tools Used

Manual code review.
https://docs.soliditylang.org/en/v0.8.10/080-breaking-changes.html

Recommended Mitigation Steps

Consider updating the smart contract suite to use the latest solidity version or at the very least integrate OpenZeppelin's SafeMath library in all areas of the code containing arithmetic operations.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 30, 2021
code423n4 added a commit that referenced this issue Nov 30, 2021
@0xScotch 0xScotch added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Dec 8, 2021
@0xScotch 0xScotch added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 8, 2021
@GalloDaSballo
Copy link
Collaborator

Lol

@GalloDaSballo GalloDaSballo added 0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 18, 2022
@GalloDaSballo
Copy link
Collaborator

In lack of:

  • poc
  • detailed bug report
  • specifics

Because solidity version is not a guarantee of a lack of security, because the sponsor has been using safeMath everywhere, I'm downgrading to non-critical

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants