ERC20Facet: permit() allows zero address spender

[adamgall]

Description

The permit() function (EIP-2612 gasless approvals) does not validate that _spender != address(0) before setting the allowance, creating an inconsistency with the standard approve() function which correctly validates.

Impact

• Severity: Medium
• Allows gasless approvals to address(0) via signature
• Inconsistent with approve() behavior in the same contract
• While signature validation makes this unlikely to occur accidentally, it's a spec violation
• Creates potential for confusion and integration issues

Current Behavior

function permit(..., address _spender, ...) external {
    // Validates deadline ✅
    if (block.timestamp > _deadline) { revert ... }

    // Validates signature ✅
    address signer = ecrecover(hash, _v, _r, _s);
    if (signer != _owner || signer == address(0)) { revert ... }

    // Sets allowance WITHOUT validating spender ❌
    s.allowances[_owner][_spender] = _value;
    emit Approval(_owner, _spender, _value);
}

Expected Behavior

Should validate spender before signature verification for consistency:

function permit(..., address _spender, ...) external {
    if (_spender == address(0)) {
        revert ERC20InvalidSpender(address(0));
    }
    // ... rest of validation
}

Comparison

// approve() - validates spender ✅
function approve(address _spender, uint256 _value) external {
    if (_spender == address(0)) {
        revert ERC20InvalidSpender(address(0));
    }
    s.allowances[msg.sender][_spender] = _value;
}

// permit() - does NOT validate spender ❌
function permit(..., address _spender, ...) external {
    // ... signature verification ...
    s.allowances[_owner][_spender] = _value;  // No validation!
}

[mudgen]

Yes, good catch.