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

fix: return error if amount is greater than 2^128 #681

Merged
merged 7 commits into from
Mar 20, 2023
Merged

Conversation

0x3bfc
Copy link
Contributor

@0x3bfc 0x3bfc commented Feb 8, 2023

Description

The exitToNear and exitToEthereum precompiles may panic if the input amount is greater than 2^128 when cast from a U256 to u128 .

engine-precompiles/src/native.rs Outdated Show resolved Hide resolved
engine-precompiles/src/native.rs Outdated Show resolved Hide resolved
engine-precompiles/src/native.rs Outdated Show resolved Hide resolved
@0x3bfc 0x3bfc marked this pull request as ready for review February 8, 2023 08:54
Copy link
Contributor

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, add test that triggers these errors please inline.

@aleksuss
Copy link
Member

aleksuss commented Feb 8, 2023

@0x3bfc for writing tests for this I'd suggest to create helper function like this:

fn validate_amount(amount: U256) -> Result<(), ExitError> {
    if amount > U256::from(u128::MAX) {
        return Err(ExitError::Other(Cow::from("ERR_INVALID_AMOUNT")));
    }

    Ok(())
}

And then you'll be able to cover the function with tests.

Copy link
Contributor

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of validate_amount here. Thanks.

@joshuajbouw
Copy link
Contributor

Before I merge it, you need to sign all the commits with your @aurora.dev e-mail. Really important.

@Aurora-ClickUp
Copy link

@aleksuss aleksuss enabled auto-merge March 20, 2023 12:31
@aleksuss aleksuss added this pull request to the merge queue Mar 20, 2023
@aleksuss aleksuss merged commit 8593642 into develop Mar 20, 2023
@aleksuss aleksuss deleted the fix/audit-aur-01 branch March 20, 2023 12:31
lempire123 pushed a commit that referenced this pull request Apr 5, 2023
* fix: return error if amount is greater than `2^128`

* Update engine-precompiles/src/native.rs

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>

* Update engine-precompiles/src/native.rs

* chore: remove unused constant

* chore: should panic with error msg if amount > 2^128

* fix: remove `validate_amount` from tests

---------

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
@joshuajbouw joshuajbouw mentioned this pull request Apr 5, 2023
birchmd pushed a commit that referenced this pull request Apr 5, 2023
* fix: return error if amount is greater than `2^128`

* Update engine-precompiles/src/native.rs

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>

* Update engine-precompiles/src/native.rs

* chore: remove unused constant

* chore: should panic with error msg if amount > 2^128

* fix: remove `validate_amount` from tests

---------

Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
joshuajbouw added a commit that referenced this pull request Apr 5, 2023
## [2.9.0] 2023-04-05

### Added

- Enabled XCC for mainnet release by [@birchmd]. ([#694])
- Added `set_owner` contract method which sets the owner of the contract
by [@hskang9]. ([#690])
- New variant of submit function `submit_with_args` which accepts
additional arguments along with the transaction such as the max gas
price a user is ready to pay by [@aleksuss]. ([#696])
- Added the ability to create and fund XCC sub-accounts from external
NEAR accounts by [@birchmd]. ([#735])

### Changes

- Replaced `rjson` with `serde_json` by [@aleksuss]. ([#677])
- Changed owner intended contract methods to now require owner or the
contract itself by [@hskang9]. ([#676])

### Fixes

- Fixed nonce incorrectly being incremented on an out of fund failure by
[@joshuajbouw]. ([#671])
- Fixed a check in promise results before executing cross contract calls
(XCC) callbacks by [@birchmd]. ([#693])
- Fixed a reachable panic in `receive_erc20_tokens` by [@0x3bfc].
([#709])
- Fixed a lack of minimum size checks when instantiating a new `EthGas`
object by [@lempire123]. ([#722])
- Fixed a lack of division by 0 checks in `EthGas::Div()` by
[@lempire123]. ([#718])
- Fixed the validation of the return of `exports:storage_remove` by
[@0x3bfc]. ([#712])
- Fixed missing account validations of NEAR account IDs by [@0x3bfc].
([#703])
- Fixed a reachable panic in the `exitToNear` and `exitToEthereum`
precompiles if the input amount is greater than 1^128 when cast from a
`U256` to `u128` by [@0x3bfc]. ([#681])
- Fixed a reachable panic in `modExp` due to arithmetic overflow by
[@0x3bfc]. ([#688])
- Fixed the ability attaching values to Aurora specific precompiles,
this no longer is possible, by [@0x3bfc]. ([#714])
- Fixed a return error if an ecrecover signature length is not exactly
65 by [@0x3bfc]. ([#717])
- Fixed size checks on input array passed to `exitToNear` and
`exitToEthereum` precompiles by [@0x3bfc]. ([#684])
- Fixed missing gas costs in `exitToNear` and `exitToEthereum`
precompiles by [@lempire123]. ([#687])
- Fixed a reachable panic due to out of memory in the `modExp`
precompile by [@0x3bfc]. ([#689])
- Fixed an assurance that the `sender_id` has a balance greater than the
amount in `ft_transfer_call` by [@0x3bfc]. ([#708])
- Fixed returning `0x` when a length cannot be cast as `usize` instead
of returning an error in the `modExp` precompile by [@birchmd]. ([#737])
- Miscellaneous minor fixes by [@0x3bfc]. ([#738])

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Leandro Casuso Montero <leandro.montero@aurora.dev>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Alexey Lapitsky <lex@realisticgroup.com>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Alexey Lapitsky <alexey.lapitsky@aurora.dev>
Co-authored-by: Hyungsuk Kang <hskang9@gmail.com>
Co-authored-by: Ahmed Ali <ahmedaabdulwahed@gmail.com>
Co-authored-by: lempire123 <61431140+lempire123@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants