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

sway compiler doesn't prevent function selector collisions #6334

Open
IGI-111 opened this issue Jul 30, 2024 · 0 comments · May be fixed by #6869
Open

sway compiler doesn't prevent function selector collisions #6334

IGI-111 opened this issue Jul 30, 2024 · 0 comments · May be fixed by #6869
Assignees
Labels
audit-report Related to the audit report bug Something isn't working

Comments

@IGI-111
Copy link
Contributor

IGI-111 commented Jul 30, 2024

From https://bugs.immunefi.com/dashboard/submission/32835

Brief/Intro

The Sway compiler doesn't prevent function selector collisions. If two ABI functions share the first 4B in the hash of their signature, then it's undefined which of them will be called. This can result in unintended actions such as transferring tokens or renouncing ownership.

Vulnerability Details

The compiler with the v0 encoding uses 4B of the signature's hash as the function selector (similarly to the ABI defined by Solidity). When an external call is made, the function that will be executed is defined by this selector. We walk the selectors stored in the preamble and compare them with the input selector. If a match is made, then we jump to the corresponding function.

However, unlike Solidity or Vyper, Sway doesn't check for selector collisions. That means if 2 or more functions share the same selector, then the dispatch mechanism is broken because we dispatch on the first match.

Consider the attached PoC, which demonstrates unintentionally renouncing the ownership. In the test, we call the function way, but as it can be seen from the assert, the function never gets called, and instead, function fpeu is called. fpeu gets called because it has the same selector as way.

We modified the compiler to print the selector values:

    pub fn to_fn_selector_value(
        &self,
        handler: &Handler,
        engines: &Engines,
    ) -> Result<[u8; 4], ErrorEmitted> {
        let hash = self.to_fn_selector_value_untruncated(handler, engines)?;
        // 4 bytes truncation via copying into a 4 byte buffer
        let mut buf = [0u8; 4];
        buf.copy_from_slice(&hash[..4]);
        println!("selector after hashing: {:?}", buf);
sig to be hashed: "fpeu()"
selector after hashing: [12, 170, 84, 33]
sig to be hashed: "way()"
selector after hashing: [12, 170, 84, 33]

And indeed, we have the same values.

Impact Details

If a collision occurs, then the impact can be critical and depends on the actual logic of the corresponding smart contract. It can result in unintentionally sending funds to the wrong address or renouncing ownership.

If we consider sha256 as a random oracle, truncation to 4B, and the birthday paradox, we can see that we need about (2^32)^0.5=2^16=65536 hashes to get a 50% probability of a collision.

Proof of Concept

To run the PoC, start forc from the root of the project: forc test --no-encoding-v1

Forc.toml

[project]
authors = ["cyber"]
entry = "main.sw"
license = "Apache-2.0"
name = "counter-contract"

[dependencies]
sway_libs = { git = "https://github.com/FuelLabs/sway-libs", tag = "v0.22.0" }
standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.5.1" }

src/main.sw

contract;

use sway_libs::ownership::*;
use sway_libs::ownership::_owner;
use standards::src5::{SRC5, State};


abi MyContract {
    fn way() -> (bool);
    #[storage(read, write)]
    fn fpeu() -> (bool);
    #[storage(read, write)]
    fn renounce();
    #[storage(read, write)]
    fn init();
}

impl SRC5 for Contract {
    #[storage(read)]
    fn owner() -> State {
        _owner()
    }
}

impl MyContract for Contract {
    fn way() -> bool {
        // arbitrary logic
        true
    }

    #[storage(read, write)]
    fn renounce() {
        renounce_ownership();
    }

    #[storage(read, write)]
    fn fpeu() -> bool {
        // arbitrary logic
        let caller = abi(MyContract, ContractId::this().into());
        caller.renounce();
        false
    }

    #[storage(read, write)]
    fn init() {
        let id = Identity::ContractId((ContractId::this()));
        initialize_ownership(id);
    } 

}

#[test]
fn test_collision() {
    let caller = abi(MyContract, CONTRACT_ID);
    let owner_caller = abi(SRC5, CONTRACT_ID);
    let id = Identity::Address(Address::from(CONTRACT_ID));
    caller.init();
    let result = caller.way();
    assert(result == false);
    assert(owner_caller.owner() == State::Revoked);
}

To find the collision we used the following script:

import hashlib
import itertools
import string

def sha256_first_4bytes(s):
    return hashlib.sha256(s.encode()).digest()[:4]

def generate_strings():
    for length in range(1, 6):  # Adjust range for longer strings
        for chars in itertools.product(string.ascii_lowercase, repeat=length):
            yield ''.join(chars) + '()'

collisions = {}

for s in generate_strings():
    hash_prefix = sha256_first_4bytes(s)
    if hash_prefix in collisions:
        print(f"Collision found: {s} and {collisions[hash_prefix]}")
        print(f"SHA256 prefix: {hash_prefix.hex()}")
        break
    collisions[hash_prefix] = s
else:
    print("No collisions found in the given range.")
@IGI-111 IGI-111 self-assigned this Jul 30, 2024
@IGI-111 IGI-111 added bug Something isn't working audit-report Related to the audit report labels Jul 30, 2024
@IGI-111 IGI-111 assigned tritao and jjcnn and unassigned IGI-111 and tritao Jul 30, 2024
@jjcnn jjcnn linked a pull request Jan 29, 2025 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Related to the audit report bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants