ghsa-mr6r-mvw4-736g
Vulnerability from github
VVE-2020-0001
Earlier today, we received a responsible disclosure of a potential issue from @montyly (security researcher at @trailofbits) for Vyper users who make assumptions about what values certain interface types can return.
Impact
We determined the issue to be mild and unlikely to be exploited, with an easy workaround while the correct resolution is in process. The issue stems from a number of things, which we will detail here.
(1) The ABI Specification is under-defined such that function return type is not always reflected in how you use it
This means that a function which returns uint8 under the hood actually returns a 32 byte integer, making it identical to a function that returns uint256. This allows users to read an interface that returns a uint8 value to be stored into a uint256 variable without any explicit casting or input validation.
(2) Vyper doesn't have uint8 types
When Vyper was originally created, it only had one numeric type, but we added just enough types to be able to work with the majority of ERC interfaces that exist.
Unfortunately, we never added uint8, because it's only majority usage was for ERC20.decimals() as the return type, which isn't reflected in the method ID. Because of (1), it didn't matter that we didn't have these types implemented because you could capture the return value as uint256 and use it just fine.
(3) ERC20.decimals() returns uint8
ERC20.decimal() (which is an optional function) returns a uint8 type. While it was never intentioned to be used directly within a smart contract (hence being optional), someone could easily make the decision to rely on it to perform important functionality within their Vyper smart contract. This might lead to a scenario where an unexpectedly large value (> 255) returned by calling this function (which a malicious contract writer might write) would allow an attacker to manipulate or bypass certain logic depending on this value.
In summary, because of (1), it isn't necessary to have to cast the return value of a function that returns uint8 to uint256, and because of (2) it isn't possible to have the type system protect against this type of error. This could lead to scenarios like (3) where this behavior can be exploited.
Patches
We are currently refactoring our typing system so we can implement all ABI-compliant integer types, but no currently patched version is available that gives users access to the uint8 type.
Workarounds
There is an easy workaround where you should check that the value returned by an interface which specifies uint8 should be checked to be within the bounds of a uint8 integer. As an example:
```python ...
returns uint8, but we implicitly cast to uint256 without checking
decimals: uint256 = ERC20(_token).decimal()
FIX: Insert this line
assert decimals < 256 ... ```
Depending on how you use this value, it may not be necessary to insert this check.
References
For more information
If you have any questions or comments about this advisory: * Chat with us in our gitter * Open an issue in https://github.com/vyperlang/vyper * Email us at security@vyperlang.org
{
"affected": [
{
"package": {
"ecosystem": "PyPI",
"name": "vyper"
},
"ranges": [
{
"events": [
{
"introduced": "0"
},
{
"last_affected": "0.1.0b16"
}
],
"type": "ECOSYSTEM"
}
]
}
],
"aliases": [],
"database_specific": {
"cwe_ids": [],
"github_reviewed": true,
"github_reviewed_at": "2020-03-25T18:19:55Z",
"nvd_published_at": null,
"severity": "LOW"
},
"details": "# VVE-2020-0001\n\nEarlier today, we received a responsible disclosure of a potential issue from @montyly (security researcher at @trailofbits) for Vyper users who make assumptions about what values certain interface types can return.\n\n### Impact\nWe determined the issue to be mild and unlikely to be exploited, with an easy workaround while the correct resolution is in process. The issue stems from a number of things, which we will detail here.\n\n(1) The ABI Specification is under-defined such that function return type is not always reflected in how you use it\n\nThis means that a function which returns `uint8` under the hood actually returns a 32 byte integer, making it identical to a function that returns `uint256`. This allows users to read an interface that returns a `uint8` value to be stored into a `uint256` variable without any explicit casting or input validation.\n\n(2) Vyper doesn\u0026#39;t have `uint8` types\n\nWhen Vyper was originally created, it only had one numeric type, but we added just enough types to be able to work with the majority of ERC interfaces that exist.\n\nUnfortunately, we never added `uint8`, because it\u0026#39;s only majority usage was for `ERC20.decimals()` as the return type, which isn\u0026#39;t reflected in the method ID. Because of (1), it didn\u0026#39;t matter that we didn\u0026#39;t have these types implemented because you could capture the return value as `uint256` and use it just fine.\n\n(3) `ERC20.decimals()` returns `uint8`\n\n`ERC20.decimal()` (which is an optional function) returns a `uint8` type. While it was never intentioned to be used directly within a smart contract (hence being optional), someone could easily make the decision to rely on it to perform important functionality within their Vyper smart contract. This might lead to a scenario where an unexpectedly large value (\u0026gt; 255) returned by calling this function (which a malicious contract writer might write) would allow an attacker to manipulate or bypass certain logic depending on this value.\n\nIn summary, because of (1), it isn\u0026#39;t necessary to have to cast the return value of a function that returns `uint8` to `uint256`, and because of (2) it isn\u0026#39;t possible to have the type system protect against this type of error. This could lead to scenarios like (3) where this behavior can be exploited.\n\n### Patches\nWe are currently refactoring our typing system so we can implement all ABI-compliant integer types, but no currently patched version is available that gives users access to the `uint8` type.\n\n### Workarounds\nThere is an easy workaround where you should check that the value returned by an interface which specifies `uint8` should be checked to be within the bounds of a `uint8` integer. As an example:\n\n```python\n...\n# returns uint8, but we implicitly cast to uint256 without checking\ndecimals: uint256 = ERC20(_token).decimal()\n# FIX: Insert this line\nassert decimals \u0026lt; 256\n...\n```\n\nDepending on how you use this value, it may not be necessary to insert this check.\n\n### References\n* [ABI Specification](https://solidity.readthedocs.io/en/latest/abi-spec.html)\n\n### For more information\nIf you have any questions or comments about this advisory:\n* Chat with us in [our gitter ](https://gitter.im/vyperlang/community)\n* Open an issue in [https://github.com/vyperlang/vyper](https://github.com/vyperlang/vyper)\n* Email us at [security@vyperlang.org](mailto:security@vyperlang.org)",
"id": "GHSA-mr6r-mvw4-736g",
"modified": "2020-03-25T18:19:55Z",
"published": "2020-03-25T18:20:19Z",
"references": [
{
"type": "WEB",
"url": "https://github.com/vyperlang/vyper/security/advisories/GHSA-mr6r-mvw4-736g"
}
],
"schema_version": "1.4.0",
"severity": [],
"summary": "Vyper interfaces returning integer types less than 256 bits can be manipulated if uint256 is used"
}
Sightings
| Author | Source | Type | Date |
|---|
Nomenclature
- Seen: The vulnerability was mentioned, discussed, or seen somewhere by the user.
- Confirmed: The vulnerability is confirmed from an analyst perspective.
- Published Proof of Concept: A public proof of concept is available for this vulnerability.
- Exploited: This vulnerability was exploited and seen by the user reporting the sighting.
- Patched: This vulnerability was successfully patched by the user reporting the sighting.
- Not exploited: This vulnerability was not exploited or seen by the user reporting the sighting.
- Not confirmed: The user expresses doubt about the veracity of the vulnerability.
- Not patched: This vulnerability was not successfully patched by the user reporting the sighting.