Skip to content

fix(security): resolve EIP-712 u256 signature forgery and cross-contract replay in STRK delegation (F-01, F-07) - #27

Open
magqqgq wants to merge 1 commit into
starknet-io:SN-v0.14.2from
magqqgq:magqqgq-patch-1
Open

magqqgq wants to merge 1 commit into
starknet-io:SN-v0.14.2from
magqqgq:magqqgq-patch-1

Conversation

@magqqgq

@magqqgq magqqgq commented Oct 12, 2026

Copy link
Copy Markdown

Summary of Changes

Addresses F-01 (Critical) and F-07 (High) from the Starknet Workspace Security Audit in starkgate-contracts:

  • Complete u256 Limb Commitment in EIP-712 Signatures (F-01):

    • Updated lock_and_delegate_input_hash in eip712_utils.cairo to hash both amount.low and amount.high.
    • Previously, only amount.low was hashed, allowing an attacker with a signature for amount $X$ to forge a valid delegation transaction for $X + k \cdot 2^{128}$.
    • Updated the EIP-712 type string to LockAndDelegateRequest(delegatee:felt,amount:u256,nonce:felt,expiry:felt) and recomputed LOCK_AND_DELEGATE_TYPE_HASH to 0x34747b0d2887193758ab94a43ee8a024d452a894901af5ca548b220fab47397.
  • Verifying Contract Address Binding in Domain Separator (F-07):

    • Included get_contract_address().into() in calc_domain_hash() to bind the EIP-712 domain to the specific deployed contract instance.
    • Prevents cross-contract and cross-chain replay of lock-and-delegate signatures across testnets, forks, or different deployments.
    • Recomputed STARKNET_DOMAIN_TYPE_HASH to 0xca98cfb7dd7f8905a216be4121e5183d688a00106289a1547b8bc15b38461.
  • Spender Validation Hardening (F-20):

    • Added assert(spender.is_non_zero(), Errors::ZERO_ADDRESS); in approve_l1_address to prevent zero-address approvals during rescue operations.
  • Allowance Saturation:

    • In _increase_account_allowance, saturated the allowance to Bounded::MAX if current_allowance + amount exceeds u256::MAX.

Operational & Deployment Notice

Breaking Change: This update alters both the message and domain hashes. Existing signatures issued under the old scheme will fail validation. Contract redeployment is required because domain_hash is initialized in the constructor and stored immutably in contract storage. Off-chain keepers and front-end signers must be updated in lockstep.

Affected Files

  • packages/strk/src/eip712_utils.cairo
  • packages/strk/src/erc20_lockable.cairo

Verification

# Verify build and integration tests
scarb --release build
snforge test

…act replay in STRK delegation (F-01, F-07)

### Summary of Changes
Addresses **F-01 (Critical)** and **F-07 (High)** from the Starknet Workspace Security Audit in `starkgate-contracts`:

- **Complete `u256` Limb Commitment in EIP-712 Signatures (F-01):**
  - Updated `lock_and_delegate_input_hash` in `eip712_utils.cairo` to hash both `amount.low` and `amount.high`.
  - Previously, only `amount.low` was hashed, allowing an attacker with a signature for amount $X$ to forge a valid delegation transaction for $X + k \cdot 2^{128}$.
  - Updated the EIP-712 type string to `LockAndDelegateRequest(delegatee:felt,amount:u256,nonce:felt,expiry:felt)` and recomputed `LOCK_AND_DELEGATE_TYPE_HASH` to `0x34747b0d2887193758ab94a43ee8a024d452a894901af5ca548b220fab47397`.

- **Verifying Contract Address Binding in Domain Separator (F-07):**
  - Included `get_contract_address().into()` in `calc_domain_hash()` to bind the EIP-712 domain to the specific deployed contract instance.
  - Prevents cross-contract and cross-chain replay of lock-and-delegate signatures across testnets, forks, or different deployments.
  - Recomputed `STARKNET_DOMAIN_TYPE_HASH` to `0xca98cfb7dd7f8905a216be4121e5183d688a00106289a1547b8bc15b38461`.

- **Spender Validation Hardening (F-20):**
  - Added `assert(spender.is_non_zero(), Errors::ZERO_ADDRESS);` in `approve_l1_address` to prevent zero-address approvals during rescue operations.

- **Allowance Saturation:**
  - In `_increase_account_allowance`, saturated the allowance to `Bounded::MAX` if `current_allowance + amount` exceeds `u256::MAX`.

### Operational & Deployment Notice
> **Breaking Change:** This update alters both the message and domain hashes. Existing signatures issued under the old scheme will fail validation. Contract redeployment is required because `domain_hash` is initialized in the constructor and stored immutably in contract storage. Off-chain keepers and front-end signers must be updated in lockstep.

### Affected Files
- `packages/strk/src/eip712_utils.cairo`
- `packages/strk/src/erc20_lockable.cairo`

### Verification
```bash
# Verify build and integration tests
scarb --release build
snforge test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant