[Security Review] Two vulnerabilities in GitVault — arbitrary dexRouter drain + relayer sig replay

[forgou37]

Security Review — GitVault.sol

Hello gitbank team,

I reviewed the GitVault.sol source and found two concrete vulnerabilities before reaching out. This is unsolicited but offered in good faith — no reward requested, just offering a free full report if you find the below useful.

---

Finding 1 — HIGH: gitSwap accepts arbitrary dexRouter with no whitelist

File: src/GitVault.sol — gitSwap() (L387–L438)

IERC20(tokenIn).approve(dexRouter, swapAmount);  // L421
uint256 outBefore = IERC20(tokenOut).balanceOf(address(this));
(bool success, ) = dexRouter.call(routerData);   // L423 — arbitrary external call
require(success, "GitVault: DEX call failed");
IERC20(tokenIn).approve(dexRouter, 0);
uint256 received = IERC20(tokenOut).balanceOf(address(this)) - outBefore;
require(received >= minAmountOut, "GitVault: slippage exceeded");

Issue: dexRouter and routerData are fully unconstrained parameters. Only tokenOut is whitelisted — the router address itself is not.

Attack path (requires compromised or malicious relayer + owner key):

1. Deploy MaliciousRouter that on-call uses the vault's ERC-20 approval to transferFrom(vault, attacker, swapAmount) and returns 1 wei of WETH to the vault.
2. Call gitSwap(tokenIn=USDC, tokenOut=WETH, amountIn=X, minAmountOut=1, dexRouter=MaliciousRouter, ...)
3. received >= 1 passes — vault's USDC is fully drained.

Why this matters: The Gitbank server generates both the owner keypair (AES-256-GCM encrypted off-chain) and the relayerSig. A single server-side compromise is sufficient to exploit this — no on-chain brute force needed. The contract's dual-sig model is undermined by the unconstrained router.

Fix: Maintain an on-chain whitelist of approved DEX router addresses, set at initialize() and updateable only via the dual-sig mechanism.

---

Finding 2 — MEDIUM: Relayer signature not bound to operation type or nonce

File: src/GitVault.sol — requireRelayerAuth modifier (L215–L221)

modifier requireRelayerAuth(uint256 deadline, bytes calldata sig) {
    require(block.timestamp <= deadline, "GitVault: relayer sig expired");
    bytes32 hash = keccak256(abi.encodePacked(address(this), githubUserId, deadline));
    // ^ only covers: vault address, githubUserId, deadline
    // NOT covered: operation type, nonce, token, amount, destination
    address recovered = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(hash), sig);
    require(recovered == relayerSigner, "GitVault: invalid relayer sig");
    _;
}

Issue: A single relayer sig for deadline T authorizes any operation on that vault within the 5-minute window — gitShield, gitUnshield, gitSwap, finalizeTransfer, createProject, etc. The relayer sig is fungible across all operations sharing the same deadline.

Scenario:

• Relayer issues sig for deadline T to authorize gitShield.
• If an attacker holds a valid owner-sig for a different operation at the same deadline T (e.g., gitUnshield to attacker-controlled address), they can submit the unauthorized operation using the intercepted relayer sig.
• During high-activity periods, multiple operations may legitimately share the same deadline window.

Fix: Bind the relayer sig to the specific operation:

bytes32 hash = keccak256(abi.encodePacked(
    address(this), githubUserId, bytes4(operationSelector), expectedNonce, deadline
));

---

Minor observations

• initTransfer() lacks nonReentrant (only finalizeTransfer has it)
• finalizeTransfer sends to any to address — not enforced to be a GitVault despite the documentation calling it a "recipient vault"
• Emergency withdraw relies on the owner key remaining intact for 6 months — if the key is compromised during that window, there is no recourse

---

If you'd like a full written report with PoC code for Finding 1, I can provide it — no strings attached. I'm a smart contract security researcher (nullref on Cantina/Code4rena).

Feel free to respond here or simply close this if the issues are already addressed internally.