fix: ensure hex values are prefixed or fail (#1454)

* change strictness * adjust tests * add codes * add checks * tests * catch hexlify case * remove prefixing * use slice * cs * prefix * revert * adjust how the API works * fix address
FuelLabs · Nov 27, 2023 · c9a8b33 · c9a8b33
1 parent 25cc508
commit c9a8b33
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 18 deletions.
diff --git a/.changeset/strong-penguins-pretend.md b/.changeset/strong-penguins-pretend.md
@@ -0,0 +1,7 @@
+---
+"@fuel-ts/address": minor
+"@fuel-ts/contract": minor
+"@fuel-ts/errors": patch
+---
+
+Remove hexlify logic on values that are not hex
diff --git a/packages/address/src/address.test.ts b/packages/address/src/address.test.ts
@@ -87,9 +87,9 @@ describe('Address utils', () => {
   });
 
   test('isB256 (no hex prefix)', () => {
-    const result = utils.isB256('ef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a');
+    const result = utils.isB256(ADDRESS_B256.slice(2));
 
-    expect(result).toBeTruthy();
+    expect(result).toBeFalsy();
   });
 
   test('isB256 (using toB256)', () => {
@@ -123,11 +123,9 @@ describe('Address utils', () => {
   });
 
   test('isPublicKey (no hex prefix)', () => {
-    const result = utils.isPublicKey(
-      '2f34bc0df4db0ec391792cedb05768832b49b1aa3a2dd8c30054d1af00f67d00b74b7acbbf3087c8e0b1a4c343db50aa471d21f278ff5ce09f07795d541fb47e'
-    );
+    const result = utils.isPublicKey(PUBLIC_KEY.slice(2));
 
-    expect(result).toBeTruthy();
+    expect(result).toBeFalsy();
   });
 
   test('isEvmAddress (EvmAddress)', () => {
@@ -155,9 +153,9 @@ describe('Address utils', () => {
   });
 
   test('isEvmAddress (no hex prefix)', () => {
-    const result = utils.isEvmAddress('07a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a');
+    const result = utils.isEvmAddress(ADDRESS_EVM.slice(2));
 
-    expect(result).toBeTruthy();
+    expect(result).toBeFalsy();
   });
 
   test('getBytesFromBech32 (bech32 to Uint8Array)', () => {
@@ -256,13 +254,33 @@ describe('Address class', () => {
     expect(address.toB256()).toEqual(signMessageTest.b256Address);
   });
 
+  test('create an Address class using invalid public key (no hex prefix)', async () => {
+    const address = PUBLIC_KEY.slice(2);
+
+    const expectedError = new FuelError(
+      FuelError.CODES.INVALID_PUBLIC_KEY,
+      `Invalid Public Key: ${address}.`
+    );
+    await expectToThrowFuelError(() => Address.fromPublicKey(address), expectedError);
+  });
+
   test('create an Address class using b256Address', () => {
     const address = Address.fromB256(ADDRESS_B256);
 
     expect(address.toAddress()).toEqual(ADDRESS_BECH32);
     expect(address.toB256()).toEqual(ADDRESS_B256);
   });
 
+  test('create an Address class using invalid b256Address (no hex prefix)', async () => {
+    const address = ADDRESS_B256.slice(2);
+
+    const expectedError = new FuelError(
+      FuelError.CODES.INVALID_B256_ADDRESS,
+      `Invalid B256 Address: ${address}.`
+    );
+    await expectToThrowFuelError(() => Address.fromB256(address), expectedError);
+  });
+
   test('when parsing to JSON it should show the bech32 address', () => {
     const result = Address.fromB256(signMessageTest.b256Address);
     expect(JSON.stringify(result)).toEqual(`"${signMessageTest.address}"`);
@@ -334,4 +352,14 @@ describe('Address class', () => {
     expect(address.toEvmAddress()).toMatchObject(evmAddressWrapped);
     expect(address.toB256()).toEqual(ADDRESS_B256_EVM_PADDED);
   });
+
+  test('create an Address class using invalid Evm Address (no hex prefix)', async () => {
+    const address = ADDRESS_EVM.slice(2);
+
+    const expectedError = new FuelError(
+      FuelError.CODES.INVALID_EVM_ADDRESS,
+      `Invalid Evm Address: ${address}.`
+    );
+    await expectToThrowFuelError(() => Address.fromEvmAddress(address), expectedError);
+  });
 });
diff --git a/packages/address/src/address.ts b/packages/address/src/address.ts
@@ -134,6 +134,10 @@ export default class Address extends AbstractAddress {
    * @returns A new `Address` instance
    */
   static fromPublicKey(publicKey: string): Address {
+    if (!isPublicKey(publicKey)) {
+      throw new FuelError(FuelError.CODES.INVALID_PUBLIC_KEY, `Invalid Public Key: ${publicKey}.`);
+    }
+
     const b256Address = sha256(hexlify(getBytesCopy(publicKey)));
     return new Address(toBech32(b256Address));
   }
@@ -145,6 +149,13 @@ export default class Address extends AbstractAddress {
    * @returns A new `Address` instance
    */
   static fromB256(b256Address: string): Address {
+    if (!isB256(b256Address)) {
+      throw new FuelError(
+        FuelError.CODES.INVALID_B256_ADDRESS,
+        `Invalid B256 Address: ${b256Address}.`
+      );
+    }
+
     return new Address(toBech32(b256Address));
   }
 
@@ -218,6 +229,13 @@ export default class Address extends AbstractAddress {
    * @returns A new `Address` instance
    */
   static fromEvmAddress(evmAddress: string): Address {
+    if (!isEvmAddress(evmAddress)) {
+      throw new FuelError(
+        FuelError.CODES.INVALID_EVM_ADDRESS,
+        `Invalid Evm Address: ${evmAddress}.`
+      );
+    }
+
     const paddedAddress = padFirst12BytesOfEvmAddress(evmAddress);
 
     return new Address(toBech32(paddedAddress));

diff --git a/packages/address/src/utils.ts b/packages/address/src/utils.ts
@@ -61,7 +61,7 @@ export function isBech32(address: BytesLike): boolean {
  * @hidden
  */
 export function isB256(address: string): boolean {
-  return (address.length === 66 || address.length === 64) && /(0x)?[0-9a-f]{64}$/i.test(address);
+  return address.length === 66 && /(0x)[0-9a-f]{64}$/i.test(address);
 }
 
 /**
@@ -70,7 +70,7 @@ export function isB256(address: string): boolean {
  * @hidden
  */
 export function isPublicKey(address: string): boolean {
-  return (address.length === 130 || address.length === 128) && /(0x)?[0-9a-f]{128}$/i.test(address);
+  return address.length === 130 && /(0x)[0-9a-f]{128}$/i.test(address);
 }
 
 /**
@@ -79,7 +79,7 @@ export function isPublicKey(address: string): boolean {
  * @hidden
  */
 export function isEvmAddress(address: string): boolean {
-  return (address.length === 42 || address.length === 40) && /(0x)?[0-9a-f]{40}$/i.test(address);
+  return address.length === 42 && /(0x)[0-9a-f]{40}$/i.test(address);
 }
 
 /**
@@ -187,6 +187,5 @@ export const padFirst12BytesOfEvmAddress = (address: string): B256AddressEvm =>
     throw new FuelError(FuelError.CODES.INVALID_EVM_ADDRESS, 'Invalid EVM address format.');
   }
 
-  const prefixedAddress = address.startsWith('0x') ? address : `0x${address}`;
-  return prefixedAddress.replace('0x', '0x000000000000000000000000') as B256AddressEvm;
+  return address.replace('0x', '0x000000000000000000000000') as B256AddressEvm;
 };
diff --git a/packages/contract/src/contract-factory.ts b/packages/contract/src/contract-factory.ts
@@ -92,8 +92,8 @@ export default class ContractFactory {
   createTransactionRequest(deployContractOptions?: DeployContractOptions) {
     const storageSlots = deployContractOptions?.storageSlots
       ?.map(({ key, value }) => ({
-        key: hexlifyWithPrefix(key),
-        value: hexlifyWithPrefix(value),
+        key: hexlifyWithPrefix(key, true),
+        value: hexlifyWithPrefix(value, true),
       }))
       .sort(({ key: keyA }, { key: keyB }) => keyA.localeCompare(keyB));
 

diff --git a/packages/contract/src/util.ts b/packages/contract/src/util.ts
@@ -1,3 +1,4 @@
+import { FuelError } from '@fuel-ts/errors';
 import { calcRoot, SparseMerkleTree } from '@fuel-ts/merkle';
 import type { StorageSlot } from '@fuel-ts/transactions';
 import { chunkAndPadBytes } from '@fuel-ts/utils';
@@ -63,11 +64,17 @@ export const getContractId = (
  * Ensures that a string is hexlified.
  *
  * @param value - The value to be hexlified.
+ * @param isKnownHex - Required if using hex values that need to be converted
  * @returns The input value hexlified with prefix.
  */
-export const hexlifyWithPrefix = (value: string) => {
+export const hexlifyWithPrefix = (value: string, isKnownHex = false) => {
   if (value.startsWith('0x')) {
     return hexlify(value);
   }
-  return hexlify(`0x${value}`);
+
+  if (isKnownHex) {
+    return hexlify(`0x${value}`);
+  }
+
+  throw new FuelError(FuelError.CODES.UNEXPECTED_HEX_VALUE, `Value should be hex string ${value}.`);
 };
diff --git a/packages/errors/src/error-codes.ts b/packages/errors/src/error-codes.ts
@@ -22,6 +22,7 @@ export enum ErrorCode {
   // address
   INVALID_BECH32_ADDRESS = 'invalid-bech32-address',
   INVALID_EVM_ADDRESS = 'invalid-evm-address',
+  INVALID_B256_ADDRESS = 'invalid-b256-address',
 
   // provider
   INVALID_URL = 'invalid-url',
@@ -30,6 +31,7 @@ export enum ErrorCode {
   MISSING_PROVIDER = 'missing-provider',
 
   // wallet
+  INVALID_PUBLIC_KEY = 'invalid-public-key',
   INSUFFICIENT_BALANCE = 'insufficient-balance',
   WALLET_MANAGER_ERROR = 'wallet-manager-error',
   HD_WALLET_ERROR = 'hd-wallet-error',
@@ -47,6 +49,7 @@ export enum ErrorCode {
   CONVERTING_FAILED = 'converting-error',
   ELEMENT_NOT_FOUND = 'element-not-found',
   MISSING_REQUIRED_PARAMETER = 'missing-required-parameter',
+  UNEXPECTED_HEX_VALUE = 'unexpected-hex-value',
 
   // transaction
   GAS_PRICE_TOO_LOW = 'gas-price-too-low',

diff --git a/packages/wallet/src/account.test.ts b/packages/wallet/src/account.test.ts
@@ -249,7 +249,9 @@ describe('Account', () => {
   it('should execute transfer just as fine', async () => {
     const amount = bn(1);
     const assetId = '0x0101010101010101010101010101010101010101010101010101010101010101';
-    const destination = Address.fromAddressOrString('0x0101010101010101010101010101010101010101');
+    const destination = Address.fromAddressOrString(
+      '0x0101010101010101010101010101010101010101000000000000000000000000'
+    );
     const txParam: Pick<TransactionRequestLike, 'gasLimit' | 'gasPrice' | 'maturity'> = {
       gasLimit: bn(1),
       gasPrice: bn(1),