Skip to content

Commit

Permalink
Release 2.5.3. (#527)
Browse files Browse the repository at this point in the history
* Fix(precompile): ExitToNear ExitToEthereum vulnerability patch

Fix vulnerability

Include exploit contract

* Release 2.5.3 notes

* Update solidity version

Co-authored-by: Michael Birch <[email protected]>
  • Loading branch information
joshuajbouw and birchmd committed Jun 7, 2022
1 parent 5c8691e commit f95e32f
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 6 deletions.
9 changes: 8 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [2.5.3] 2022-04-27

### Fixes

- Fixed inflation vulnerability relating to ExitToNear and ExitToEthereum by [@birchmd], [@mfornet], and [@joshuajbouw]. Written by [@birchmd].

## [2.5.2] 2022-03-22

### Removed
Expand Down Expand Up @@ -238,7 +244,8 @@ struct SubmitResult {

## [1.0.0] - 2021-05-12

[Unreleased]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.2...develop
[Unreleased]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.3...develop
[2.5.3]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.2...2.5.3
[2.5.2]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.1...2.5.2
[2.5.1]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.0...2.5.1
[2.5.0]: https://github.com/aurora-is-near/aurora-engine/compare/2.4.0...2.5.0
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ documentation.

Network | Contract ID | Chain ID | Version
------- | ------------------- | ---------- | ------
Mainnet | [`aurora`][Mainnet] | 1313161554 | 2.4.0
Testnet | [`aurora`][Testnet] | 1313161555 | 2.5.1
Local | `aurora.test.near` | 1313161556 | 2.5.1
Mainnet | [`aurora`][Mainnet] | 1313161554 | 2.5.3
Testnet | [`aurora`][Testnet] | 1313161555 | 2.5.3
Local | `aurora.test.near` | 1313161556 | 2.5.3

[Mainnet]: https://explorer.near.org/accounts/aurora
[Testnet]: https://explorer.testnet.near.org/accounts/aurora
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.5.2
2.5.3
4 changes: 4 additions & 0 deletions engine-precompiles/src/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ impl Precompile for ExitToNear {
// It's not allowed to call exit precompiles in static mode
if is_static {
return Err(ExitError::Other(Cow::from("ERR_INVALID_IN_STATIC")));
} else if context.address != Self::ADDRESS.raw() {
return Err(ExitError::Other(Cow::from("ERR_INVALID_IN_DELEGATE")));
}

// First byte of the input is a flag, selecting the behavior to be triggered:
Expand Down Expand Up @@ -473,6 +475,8 @@ impl Precompile for ExitToEthereum {
// It's not allowed to call exit precompiles in static mode
if is_static {
return Err(ExitError::Other(Cow::from("ERR_INVALID_IN_STATIC")));
} else if context.address != Self::ADDRESS.raw() {
return Err(ExitError::Other(Cow::from("ERR_INVALID_IN_DELEGATE")));
}

// First byte of the input is a flag, selecting the behavior to be triggered:
Expand Down
65 changes: 64 additions & 1 deletion engine-tests/src/tests/erc20_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ mod sim_tests {
CallArgs, DeployErc20TokenArgs, FunctionCallArgsV2, SubmitResult,
};
use aurora_engine_types::types::Address;
use borsh::BorshSerialize;
use borsh::{BorshDeserialize, BorshSerialize};
use near_sdk_sim::UserAccount;
use serde_json::json;

Expand All @@ -414,6 +414,69 @@ mod sim_tests {
const INITIAL_ETH_BALANCE: u64 = 777_777_777;
const ETH_EXIT_AMOUNT: u64 = 111_111_111;

#[test]
fn test_ghsa_5c82_x4m4_hcj6_exploit() {
let TestExitToNearEthContext {
mut signer,
signer_address,
chain_id,
tester_address: _,
aurora,
} = test_exit_to_near_eth_common();

let constructor = test_utils::solidity::ContractConstructor::force_compile(
"src/tests/res",
"target/solidity_build",
"exploit.sol",
"Exploit",
);
let nonce = signer.use_nonce().into();
let deploy_tx = constructor.deploy_without_constructor(nonce);
let signed_tx = test_utils::sign_transaction(deploy_tx, Some(chain_id), &signer.secret_key);
let deploy_result = aurora.call("submit", &rlp::encode(&signed_tx));
let contract_address = match &deploy_result.status() {
near_sdk_sim::transaction::ExecutionStatus::SuccessValue(bytes) => {
let submit_result = SubmitResult::try_from_slice(bytes).unwrap();
Address::try_from_slice(test_utils::unwrap_success_slice(&submit_result)).unwrap()
}
_ => panic!("Unknown result: {:?}", deploy_result),
};
let contract = constructor.deployed_at(contract_address);

let nonce = signer.use_nonce().into();
let hacker_account = "hacker.near";
let hacker_account_bytes = hacker_account.as_bytes().to_vec();
let mut exploit_tx = contract.call_method_with_args(
"exploit",
&[ethabi::Token::Bytes(hacker_account_bytes)],
nonce,
);
exploit_tx.value = Wei::new_u64(ETH_EXIT_AMOUNT);
let signed_tx =
test_utils::sign_transaction(exploit_tx, Some(chain_id), &signer.secret_key);
aurora
.call("submit", &rlp::encode(&signed_tx))
.assert_success();

// check balances -- Hacker does not steal any funds!
assert_eq!(
nep_141_balance_of(
aurora.contract.account_id.as_str(),
&aurora.contract,
&aurora,
),
INITIAL_ETH_BALANCE as u128
);
assert_eq!(
nep_141_balance_of(hacker_account, &aurora.contract, &aurora),
0
);
assert_eq!(
eth_balance_of(signer_address, &aurora),
Wei::new_u64(INITIAL_ETH_BALANCE)
);
}

#[test]
fn test_exit_to_near() {
// Deploy Aurora; deploy NEP-141; bridge NEP-141 to ERC-20 on Aurora
Expand Down
25 changes: 25 additions & 0 deletions engine-tests/src/tests/res/exploit.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.6;

contract Exploit {
address payable private owner;

constructor() {
owner = payable(msg.sender);
}

function exploit(bytes memory recipient) public payable {
require(msg.sender == owner);

bytes memory input = abi.encodePacked("\x00", recipient);
uint input_size = 1 + recipient.length;

assembly {
let res := delegatecall(gas(), 0xe9217bc70b7ed1f598ddd3199e80b093fa71124f, add(input, 32), input_size, 0, 32)
}

owner.transfer(msg.value);
}
}

3 comments on commit f95e32f

@Q5Ca
Copy link

@Q5Ca Q5Ca commented on f95e32f Jun 11, 2022

Choose a reason for hiding this comment

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

Hi.
Can you explain how the check context.address != Self::ADDRESS.raw() can prevent delegate call?

@birchmd
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the question @Q5Ca

In the EVM, a delegate call executes the logic of the target contract in the present context (i.e. the current address and ETH value are unchanged). The logic in question here is part of a precompile with a statically known address (Self::ADDRESS; the .raw() is just making the types line up and has no relevance to the high level logic). Therefore, the condition context.address != Self::ADDRESS catches delegate calls because, in the case of a delegate call, the EVM context will be the address of the caller rather than Self::ADDRESS. If that condition is true then we immediately exit the precompile, preventing any of the precompile's important logic from being executed in the wrong context.

@Q5Ca
Copy link

@Q5Ca Q5Ca commented on f95e32f Jun 12, 2022

Choose a reason for hiding this comment

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

Hi @birchmd

Thank you for the clear explaination. I got it.

Please sign in to comment.