fix(lido validator): verify tx calldata not tampered #1

ajag408 · 2025-10-14T22:18:37Z

🔒 Security Fix: Prevent Calldata Tampering in Lido Validator

Problem

The Lido validator was vulnerable to calldata tampering attacks across all transaction types (stake, unstake, claim). The ethers.Interface.parseTransaction() method ignores extra bytes appended to the end of calldata, which means an attacker could append arbitrary data to a valid transaction and it would still pass validation.

Discovered during smoke testing: Appending deadbeef to valid stake calldata was incorrectly accepted by the validator.

Impact

Attack Vector: Appending malicious data to valid transactions
Affected Operations: STAKE, UNSTAKE, CLAIM_UNSTAKED (all Lido operations)
Risk: Data integrity compromise, potential for hidden malicious payloads, violation of zero-trust principle

Root Cause

The ethers.Interface.parseTransaction() method follows standard ABI decoding behavior where trailing bytes are ignored. After parsing, the validators only validated the decoded arguments (referral address, owner, amounts, etc.) but never verified that the original calldata matched the expected encoded length for those arguments.

Example of the vulnerability:

Valid calldata: 0xa1903eab000000000000000000000000371240e80bf84ec2ba8b55ae2fd0b467b16db2be

Tampered calldata (with deadbeef appended) - was incorrectly accepted: 0xa1903eab000000000000000000000000371240e80bf84ec2ba8b55ae2fd0b467b16db2bedeadbeef

Solution

Implemented strict calldata integrity validation using a re-encoding approach:

Added ensureCalldataNotTampered() helper in BaseEVMValidator:
- Re-encodes parsed transaction arguments using encodeFunctionData()
- Compares re-encoded calldata with original calldata byte-for-byte
- Rejects transactions where they don't match exactly
- Works for both fixed-length and dynamic-length calldata (arrays)
Added parseAndValidateCalldata() helper in LidoValidator:
- Centralizes parsing and tampering validation logic (DRY principle)
- Ensures consistent error handling across all validation methods
- Automatically applies tampering check to all parsed transactions
Applied to all Lido validation methods:
- validateStake(): Validates submit(address) calldata
- validateUnstake(): Validates requestWithdrawals(uint256[], address) calldata
- validateClaim(): Validates both claimWithdrawal(uint256) and claimWithdrawals(uint256[], uint256[]) calldata

Why Re-encoding Works

The encodeFunctionData() method produces canonical ABI encoding. Any deviation from canonical encoding is detected:

✅ Trailing bytes appended
✅ Non-standard padding
✅ Malformed array offsets
✅ Any non-canonical encoding tricks

Both ethers.js and the EVM use the same canonical encoding rules, so the re-encoded version represents exactly what the smart contract will execute.

Changes

`parent/src/validators/evm/base.validator.ts`

Added ensureCalldataNotTampered() protected helper method
Provides reusable tampering detection for all EVM validators

`parent/src/validators/evm/lido/lido.validator.ts`

Added parseAndValidateCalldata() private helper method
Refactored validateStake() to use the helper (DRY)
Refactored validateUnstake() to use the helper (DRY)
Refactored validateClaim() to use the helper (DRY)
Removed duplicate try/catch blocks and tampering checks

`parent/src/validators/evm/lido/lido.validator.test.ts`

Added comprehensive tampering detection test suite
Tests verify rejection of appended bytes (4 bytes, 32 bytes) for all transaction types
Tests verify valid transactions without extra bytes still pass
Tests cover both single values and array parameters
Tests for STAKE, UNSTAKE, CLAIM (single), and CLAIM (batch) operations

Testing

✅ All existing unit tests pass (no regressions)
✅ New unit tests verify tampering detection for STAKE, UNSTAKE, and CLAIM operations
✅ Tests confirm valid transactions without extra bytes are still accepted
✅ Smoke tests confirm Test 10 (Append 4 Bytes) now correctly blocks tampered transactions
✅ Tests cover both fixed-length calldata (stake, single claim) and dynamic-length calldata (unstake, batch claim)

Code Quality

DRY Principle: Tampering check logic centralized in parseAndValidateCalldata() helper
Maintainability: Single point of change for parsing and validation logic
Type Safety: TypeScript discriminated unions ensure proper error handling
Consistency: All validation methods follow the same pattern

Security Considerations

This fix ensures complete data integrity by validating that the exact calldata matches the canonical encoding of the parsed and validated arguments. No extra bytes can be hidden in transaction data, regardless of whether the calldata is fixed-length or dynamic-length.

Zero-Trust Principle

This change reinforces Shield's zero-trust validation philosophy:

Before: Trusted that parsed data represented the complete transaction
After: Explicitly verifies that no additional data exists beyond what was validated
Result: Complete data integrity guarantee for all Lido transactions

Future Considerations

The ensureCalldataNotTampered() method in BaseEVMValidator is now available for other EVM validators to use, providing a consistent approach to calldata integrity validation across the entire Shield library.

Unstake/Claim operations: While the re-encoding approach provides comprehensive protection, additional security review of dynamic-length calldata handling (arrays in requestWithdrawals and claimWithdrawals) is recommended to ensure no edge cases exist in complex ABI encoding scenarios

Copilot

Pull Request Overview

Security hardening to detect and block Lido validator transactions whose calldata has been tampered by appending extra bytes.

Centralizes parsing + calldata integrity check via parseAndValidateCalldata
Adds canonical re-encoding comparison (ensureCalldataNotTampered) to detect any trailing/malformed bytes
Expands test suite with tampering scenarios for all Lido operation types

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/validators/evm/base.validator.ts	Adds reusable calldata integrity helper using re-encoding comparison
src/validators/evm/lido/lido.validator.ts	Refactors stake/unstake/claim validation to use centralized parse + tamper check
src/validators/evm/lido/lido.validator.test.ts	Adds tests covering tampered calldata for stake, unstake, and claim paths

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-15T11:42:28Z

src/validators/evm/lido/lido.validator.test.ts

+    it('should reject stake transaction with appended bytes', () => {
+      const tx = {
+        to: lidoStEthAddress,
+        from: userAddress,
+        value: '0xde0b6b3a7640000',
+        data:
+          '0xa1903eab' +
+          referralAddress.slice(2).padStart(64, '0') +
+          'deadbeef', // Extra bytes appended
+        nonce: 0,
+        gasLimit: '0x30d40',
+        gasPrice: '0x4a817c800',
+        chainId: 1,
+        type: 0,
+      };
+
+      const serialized = JSON.stringify(tx);
+      const result = shield.validate({
+        yieldId,
+        unsignedTransaction: serialized,
+        userAddress,
+      });
+
+      expect(result.isValid).toBe(false);
+      expect(result.reason).toContain('No matching operation pattern found');
+      const stakeAttempt = result.details?.attempts?.find(
+        (a: any) => a.type === TransactionType.STAKE,
+      );
+      expect(stakeAttempt?.reason).toContain('calldata has been tampered');
+    });


[nitpick] Multiple tampering rejection tests (stake, unstake variants, claimWithdrawal, claimWithdrawals) duplicate the same pattern of constructing a tx, serializing, validating, and asserting on the tampered reason; consider extracting a helper (e.g., expectTampered(tx, type)) or using a table-driven test to reduce repetition and make future additions (other functions or tamper cases) simpler.

jdomingos

Amazing work! Approved but added a suggestion to prepare for future changes! 👏

jdomingos · 2025-10-15T15:17:03Z

src/validators/evm/lido/lido.validator.ts

+  /**
+   * Parse transaction and validate calldata integrity
+   */
+  private parseAndValidateCalldata(


The only thing I can think of regarding this PR is that this function could be on the base validator.

We could add the interface as a parameter and make it generic. I see this being very useful for other EVM checks!

jdomingos

👏 Congrats on this work!
Thanks for addressing my feedback!

fix(lido validator): verify tx calldata not tampered

c9dc966

ajag408 requested a review from Philippoes October 14, 2025 22:18

fix(lido-validator): linting issues

1eb5a73

ajag408 requested a review from jdomingos October 14, 2025 22:33

Philippoes requested a review from Copilot October 15, 2025 11:41

Copilot AI reviewed Oct 15, 2025

View reviewed changes

jdomingos approved these changes Oct 15, 2025

View reviewed changes

Philippoes approved these changes Oct 15, 2025

View reviewed changes

ajag408 added 2 commits October 15, 2025 19:10

fix(lido-validator): moved parseAndValidate function to evm base class

e40b0d8

fix(lido-validator): linting issues

a2c3226

jdomingos approved these changes Oct 16, 2025

View reviewed changes

Philippoes merged commit f43b9c3 into main Oct 17, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lido validator): verify tx calldata not tampered #1

fix(lido validator): verify tx calldata not tampered #1

Uh oh!

ajag408 commented Oct 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

jdomingos left a comment

Uh oh!

jdomingos Oct 15, 2025

Uh oh!

Philippoes Oct 15, 2025

Uh oh!

ajag408 Oct 16, 2025

Uh oh!

jdomingos left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix(lido validator): verify tx calldata not tampered #1

fix(lido validator): verify tx calldata not tampered #1

Uh oh!

Conversation

ajag408 commented Oct 14, 2025

🔒 Security Fix: Prevent Calldata Tampering in Lido Validator

Problem

Impact

Root Cause

Solution

Why Re-encoding Works

Changes

parent/src/validators/evm/base.validator.ts

parent/src/validators/evm/lido/lido.validator.ts

parent/src/validators/evm/lido/lido.validator.test.ts

Testing

Code Quality

Security Considerations

Zero-Trust Principle

Future Considerations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

jdomingos left a comment

Choose a reason for hiding this comment

Uh oh!

jdomingos Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Philippoes Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

ajag408 Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

jdomingos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`parent/src/validators/evm/base.validator.ts`

`parent/src/validators/evm/lido/lido.validator.ts`

`parent/src/validators/evm/lido/lido.validator.test.ts`