fix(transaction-controller): reject transactions that would deploy an empty contract (MetaMask#8744)

## Explanation

`TransactionController` would silently classify a transaction with no
recipient and no real bytecode as a contract deployment and broadcast
it, creating an empty contract on-chain with any `value` permanently
locked inside.

The root cause is that `'0x'` is a non-empty string, so `if (data)`
truthiness checks in `validateParamRecipient` and
`determineTransactionType` treated it as real deployment bytecode.
`validateParamRecipient` also didn't recognize `to === ''` as missing.

Validation now treats `to === ''` as missing alongside `'0x'` /
`undefined`, and both the validator and the type classifier require
`data.length > 2` before accepting a missing recipient as a legitimate
deployment.

## References

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Tightens core transaction validation/type inference around contract
deployments; could cause some previously-accepted malformed transactions
to be rejected or reclassified, but the change is narrowly scoped and
well-covered by tests.
> 
> **Overview**
> Prevents accidental empty contract deployments by treating missing
recipients (`to` of `''`, `'0x'`, or `undefined`) as **invalid unless**
`data` contains *real* deployment bytecode (more than just `'0x'`).
> 
> Updates both `validateTxParams`/`validateParamRecipient` and
`determineTransactionType` to require non-empty bytecode before allowing
a missing `to`, adds targeted unit tests for the new edge cases, and
records the fix in the `transaction-controller` changelog.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
7154b2f. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: matthewwalsh0

packages/transaction-controller/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Trigger the first-time-interaction warning correctly for `safeTransferFrom` token transfers by including `TransactionType.tokenMethodSafeTransferFrom` in the effective-recipient decoding logic ([#8723](https://github.com/MetaMask/core/pull/8723))
 
+### Fixed
+
+- Reject transactions with a missing `to` and empty `data` to prevent accidental empty contract deployments ([#8744](https://github.com/MetaMask/core/pull/8744))
+
 ## [65.2.0]
 
 ### Added

packages/transaction-controller/src/utils/transaction-type.test.ts

@@ -165,7 +165,7 @@ describe('determineTransactionType', () => {
     });
   });
 
-  it('returns a contract deployment type when "to" is falsy and there is data', async () => {
+  it('returns a contract deployment type when "to" is falsy and there is real bytecode', async () => {
     const result = await determineTransactionType(
       {
         ...txParams,
@@ -180,6 +180,34 @@ describe('determineTransactionType', () => {
     });
   });
 
+  it('does NOT classify as deployContract when "to" is falsy but data is "0x" (empty bytecode)', async () => {
+    jest.mocked(rpcRequest).mockResolvedValue('0x');
+
+    const result = await determineTransactionType(
+      {
+        ...txParams,
+        to: '',
+        data: '0x',
+      },
+      { messenger: messengerMock, networkClientId: networkClientIdMock },
+    );
+    expect(result.type).not.toBe(TransactionType.deployContract);
+  });
+
+  it('does NOT classify as deployContract when "to" is undefined but data is "0x"', async () => {
+    jest.mocked(rpcRequest).mockResolvedValue('0x');
+
+    const result = await determineTransactionType(
+      {
+        ...txParams,
+        to: undefined,
+        data: '0x',
+      },
+      { messenger: messengerMock, networkClientId: networkClientIdMock },
+    );
+    expect(result.type).not.toBe(TransactionType.deployContract);
+  });
+
   it('returns a simple send type with a 0x getCodeResponse when there is data, but the "to" address is not a contract address', async () => {
     jest.mocked(rpcRequest).mockResolvedValue('0x');
 

packages/transaction-controller/src/utils/transaction-type.ts

@@ -41,7 +41,9 @@ export async function determineTransactionType(
 ): Promise<InferTransactionTypeResult> {
   const { data, to } = txParams;
 
-  if (data && !to) {
+  const hasRealBytecode = Boolean(data && data !== '0x' && data.length > 2);
+
+  if (hasRealBytecode && !to) {
     return { type: TransactionType.deployContract, getCodeResponse: undefined };
   }
 

packages/transaction-controller/src/utils/validation.test.ts

@@ -77,35 +77,97 @@ describe('validation', () => {
       );
     });
 
-    it('should throw if no data', () => {
+    it('should throw if to is missing and no real bytecode', () => {
+      const expectedError = rpcErrors.invalidParams(
+        'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
+      );
+
       expect(() =>
         validateTxParams({
           from: FROM_MOCK,
           to: '0x',
           // TODO: Replace `any` with type
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } as any),
-      ).toThrow(rpcErrors.invalidParams('Invalid "to" address.'));
+      ).toThrow(expectedError);
 
       expect(() =>
         validateTxParams({
           from: FROM_MOCK,
           // TODO: Replace `any` with type
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } as any),
-      ).toThrow(rpcErrors.invalidParams('Invalid "to" address.'));
+      ).toThrow(expectedError);
+    });
+
+    it('should throw if to is empty string and no real bytecode', () => {
+      expect(() =>
+        validateTxParams({
+          from: FROM_MOCK,
+          to: '' as Hex,
+          data: '0x',
+          value: '0x1',
+          // TODO: Replace `any` with type
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        } as any),
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
+        ),
+      );
     });
 
-    it('should delete data', () => {
+    it('should throw if to is empty string and data is "0x" (would deploy empty contract)', () => {
+      expect(() =>
+        validateTxParams({
+          from: FROM_MOCK,
+          to: '' as Hex,
+          data: '0x',
+          // TODO: Replace `any` with type
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        } as any),
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
+        ),
+      );
+    });
+
+    it('should throw if to is undefined and data is "0x" (would deploy empty contract)', () => {
+      expect(() =>
+        validateTxParams({
+          from: FROM_MOCK,
+          data: '0x',
+          // TODO: Replace `any` with type
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        } as any),
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
+        ),
+      );
+    });
+
+    it('should remove "to" when missing and data has real bytecode (legitimate deployment)', () => {
       const transaction = {
-        data: 'foo',
+        data: '0x608060405234',
         from: TO_MOCK,
         to: '0x',
       };
       validateTxParams(transaction);
       expect(transaction.to).toBeUndefined();
     });
 
+    it('should remove "to" when empty string and data has real bytecode', () => {
+      const transaction = {
+        data: '0x608060405234',
+        from: TO_MOCK,
+        to: '' as Hex,
+      };
+      validateTxParams(transaction);
+      expect(transaction.to).toBeUndefined();
+    });
+
     it('should throw if invalid to address', () => {
       expect(() =>
         validateTxParams({

packages/transaction-controller/src/utils/validation.ts

@@ -205,16 +205,31 @@ function validateParamValue(value?: string): void {
  *
  * @param txParams - The transaction parameters object to validate.
  * @throws Throws an error if the recipient address is invalid:
- * - If the recipient address is an empty string ('0x') or undefined and the transaction contains data,
- * the "to" field is removed from the transaction parameters.
- * - If the recipient address is not a valid hexadecimal Ethereum address, an error is thrown.
+ * - If the recipient address is missing (empty string, '0x', or undefined) and the
+ * transaction does not contain real bytecode (data must be longer than `0x`),
+ * an error is thrown. This prevents accidental contract deployments with empty
+ * `to` and empty `data` from locking funds.
+ * - If the recipient address is missing and the transaction contains real
+ * bytecode (data longer than `0x`), the "to" field is removed from the
+ * transaction parameters (legitimate contract deployment).
+ * - If the recipient address is not a valid hexadecimal Ethereum address, an
+ * error is thrown.
  */
 function validateParamRecipient(txParams: TransactionParams): void {
-  if (txParams.to === '0x' || txParams.to === undefined) {
-    if (txParams.data) {
+  const isMissingRecipient =
+    txParams.to === '0x' || txParams.to === '' || txParams.to === undefined;
+
+  if (isMissingRecipient) {
+    const hasRealBytecode = Boolean(
+      txParams.data && txParams.data !== '0x' && txParams.data.length > 2,
+    );
+
+    if (hasRealBytecode) {
       delete txParams.to;
     } else {
-      throw rpcErrors.invalidParams(`Invalid "to" address.`);
+      throw rpcErrors.invalidParams(
+        `Invalid "to" address: must be specified for transactions without contract deployment bytecode.`,
+      );
     }
   } else if (txParams.to !== undefined && !isValidHexAddress(txParams.to)) {
     throw rpcErrors.invalidParams(`Invalid "to" address.`);