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

TOB-FUEL-14: Inconsistent encoding of vectors in ABI #1040

Closed
iqdecay opened this issue Jul 10, 2023 · 1 comment
Closed

TOB-FUEL-14: Inconsistent encoding of vectors in ABI #1040

iqdecay opened this issue Jul 10, 2023 · 1 comment
Assignees
Labels
audit-report Related to the audit report

Comments

@iqdecay
Copy link
Contributor

iqdecay commented Jul 10, 2023

Description

The encoding and decoding of vectors is inconsistent. The decoded values do not match the original values. The following test demonstrates this.

Figure 14.1: Test which demonstrates that encoding and decoding a vector results in two different vectors.

#[test]
fn inconsitent_decoding() -> Result<()> {
    let arg = Token::Vector(vec![Token::U64(42)]);
    println!("Tokens: {:?}", &arg);
    let encoded = ABIEncoder::encode(&vec![arg])?.resolve(0);
    println!("Encoded: {:?}", hex::encode(&encoded));
    let param_type = Vec::<u64>::param_type();
    println!("Type: {:?}", param_type);
    let out: Vec<Token> = ABIDecoder::decode(&[param_type], &encoded)?;
    println!("Decoded: {:?}", out);
    Ok(())
}

Figure 14.2: The output of executing the test form 14.1.

Tokens: Vector([U64(42)])
Encoded: "000000000000001800000000000000010000000000000001000000000000002a"
Type: Vector(U64)
Decoded: [Vector([U64(24), U64(1), U64(1), U64(42)])]

The reason for the difference is that the decode_vector function does not match the encode_vector function. While the encoding adds a pointer, capacity and length of the array, the decoding function interprets them as values.

Figure 14.1: Decoding function for vectors. (fuels-rs/packages/fuels-core/src/codec/abi_decoder.rs#92–100)

fn decode_vector(param_type: &ParamType, bytes: &[u8]) -> Result<DecodeResult> {
  let num_of_elements = ParamType::calculate_num_of_elements(param_type,
  bytes.len())?;
  let (tokens, bytes_read) = Self::decode_multiple(vec![param_type;
  num_of_elements], bytes)?;
      Ok(DecodeResult {
          token: Token::Vector(tokens),
          bytes_read,
      })
  }

Recommendations

Short term, change the decode_vector function such that it expects a pointer, length and capacity.
Long term, add tests which verify that the encoded value always decodes to its original value.

@iqdecay
Copy link
Contributor Author

iqdecay commented Aug 18, 2023

So it turns out that this is actually expected behavior.
Vectors can be returned from contracts only thanks to custom bytecode injection, as can be seen in

// The instructions are different if you want to return data that was on the heap
. This bytecode injection is necessary because otherwise, what the VM returns is only the pointer to the data.
It is therefore necessary that the decoding is different from the encoding.

@iqdecay iqdecay closed this as completed Aug 18, 2023
@xgreenx xgreenx changed the title fix: inconsistent Vec decoding in contracts TOB-FUEL-14: Inconsistent encoding of vectors in ABI Aug 28, 2023
@xgreenx xgreenx added the audit-report Related to the audit report label Aug 28, 2023
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
Projects
None yet
Development

No branches or pull requests

2 participants