Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Insufficient Input Validation on 'createTradingAccount' Function

juggernaut63

Summary

There is a weakness in the createTradingAccount function in the TradingAccountBranch contract that allows invalid input to be received, causing the contract to revert and fail to create a new trading account. This flaw occurs due to insufficient validation of the referralCode length.

Vulnerability Details

In the createTradingAccount function in the TradingAccountBranch contract, it was discovered that the referralCode input was not validated properly regarding its length. This condition causes the contract to revert when it receives a referralCode with a length of more than 32 bytes or is empty.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/TradingAccountBranch.sol#L229-L280

Impact

This weakness may result in the contract being unable to create a new trading account with a particular referralCode, which may ultimately compromise user experience and trust in the platform. In addition, Denial of Service (DoS) attacks can be carried out by sending invalid referralCode input repeatedly.

Tools Used

  • Manual review

  • Foundry

Recommendations

it is recommended to improve the input validation in the createTradingAccount function. Make sure the referralCode length is validated correctly before continuing the function execution.

PoC

Fuzzing with Foundry:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;
​
import "forge-std/Test.sol";
import "../src/perpetuals/branches/TradingAccountBranch.sol";
import { SD59x18, UD60x18 } from "@prb-math/SD59x18.sol";
​
contract TradingAccountBranchTest is Test {
TradingAccountBranch tradingAccountBranch;
​
function setUp() public {
tradingAccountBranch = new TradingAccountBranch();
}
​
function testFuzzingCreateTradingAccount(bytes memory referralCode, bool isCustomReferralCode) public {
emit log_bytes(referralCode);
emit log_named_uint("Referral code length", referralCode.length);
emit log_named_uint("isCustomReferralCode", isCustomReferralCode ? 1 : 0);
​
if (referralCode.length == 0 || referralCode.length > 32) {
vm.expectRevert(bytes("Referral code must be at most 32 bytes"));
}
​
try tradingAccountBranch.createTradingAccount(referralCode, isCustomReferralCode) returns (uint128 accountId) {
emit log_named_uint("Account ID", accountId);
} catch Error(string memory reason) {
emit log_string(reason);
require(false, reason);
} catch (bytes memory reason) {
emit log_bytes(reason);
require(false, "createTradingAccount failed");
}
}
​
function testFuzzingDepositMargin(uint128 tradingAccountId, address token, uint256 amount) public {
// Assume that the token has been approved to the TradingAccountBranch contract outside of this test
​
// Margin deposits
try tradingAccountBranch.depositMargin(tradingAccountId, token, amount) {
// If the deposit is successful, continue
} catch {
// If the deposit fails, we can log an error or assertion to debug
emit log("Deposit margin failed");
}
}
​
function testFuzzingWithdrawMargin(uint128 tradingAccountId, address token, uint256 amount) public {
// Assume that margin has been deposited previously
​
// Withdraw margin
try tradingAccountBranch.withdrawMargin(tradingAccountId, token, amount) {
// If the withdrawal is successful, continue
} catch {
// If the withdrawal fails, we can log an error or assertion to debug
emit log("Withdraw margin failed");
}
}
​
function testFuzzingGetAccountEquityUsd(uint128 tradingAccountId) public {
try tradingAccountBranch.getAccountEquityUsd(tradingAccountId) returns (SD59x18 equity) {
// Assertion or logging if necessary
emit log_named_int("Account Equity (USD)", equity.unwrap());
} catch {
// Logging or assertion for error handling
emit log("Get Account Equity failed");
}
}
​
function testFuzzingGetAccountLeverage(uint128 tradingAccountId) public {
try tradingAccountBranch.getAccountLeverage(tradingAccountId) returns (UD60x18 leverage) {
// Assertion or logging if necessary
emit log_named_uint("Account Leverage", leverage.unwrap());
} catch {
// Logging or assertion for error handling
emit log("Get Account Leverage failed");
}
}
}

forge test --match-path test/TradingAccountBranchTest.t.sol
[⠒] Compiling...
[⠢] Compiling 1 files with Solc 0.8.25
[⠆] Solc 0.8.25 finished in 2.03s
Compiler run successful!
proptest: Aborting shrinking after the PROPTEST_MAX_SHRINK_ITERS environment variable or ProptestConfig.max_shrink_iters iterations (set 0 t
o a large(r) value to shrink more; current configuration: 0 iterations)
Ran 5 tests for test/TradingAccountBranchTest.t.sol:TradingAccountBranchTest
[FAIL. Reason: revert: createTradingAccount failed; counterexample: calldata=0x4dd85ca700000000000000000000000000000000000000000000000000000
00000000040000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000002196 args=[0x0000000000000000000000000000000000000000000000000000000000002196, true]] testFuzzingCreateTradingAccount(bytes,bool) (runs: 0, μ: 0, ~: 0) [PASS] testFuzzingDepositMargin(uint128,address,uint256) (runs: 1001, μ: 10702, ~: 10702)
[PASS] testFuzzingGetAccountEquityUsd(uint128) (runs: 1001, μ: 9895, ~: 9895)
[PASS] testFuzzingGetAccountLeverage(uint128) (runs: 1001, μ: 9872, ~: 9872)
[PASS] testFuzzingWithdrawMargin(uint128,address,uint256) (runs: 1001, μ: 10745, ~: 10745)
Suite result: FAILED. 4 passed; 1 failed; 0 skipped; finished in 51.66ms (178.66ms CPU time)

Ran 1 test suite in 53.45ms (51.66ms CPU time): 4 tests passed, 1 failed, 0 skipped (5 total tests)

Failing tests:
Encountered 1 failing test in test/TradingAccountBranchTest.t.sol:TradingAccountBranchTest
[FAIL. Reason: revert: createTradingAccount failed; counterexample: calldata=0x4dd85ca700000000000000000000000000000000000000000000000000000
00000000040000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000002196 args=[0x0000000000000000000000000000000000000000000000000000000000002196, true]] testFuzzingCreateTradingAccount(bytes,bool) (runs: 0, μ: 0, ~: 0)
Encountered a total of 1 failing tests, 4 tests succeeded

Test with Foundry:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;
​
import "forge-std/Test.sol";
import "../src/perpetuals/branches/TradingAccountBranch.sol";
​
contract TradingAccountBranchTest is Test {
TradingAccountBranch public tradingAccountBranch;
​
function setUp() public {
tradingAccountBranch = new TradingAccountBranch();
}
​
function testReferralCodeLength() public {
// Set up a referral code that is too long
bytes memory longReferralCode = new bytes(65); // Length 65 bytes
// Doing tests
(bool success, bytes memory revertData) = address(tradingAccountBranch).call(
abi.encodeWithSignature("createTradingAccount(bytes,bool)", longReferralCode, true)
);
​
if (success) {
// If successful, then this is a failure because it should revert
revert("Expected createTradingAccount to revert due to long referral code");
} else {
// Check that the revert reason is as expected
string memory revertReason = getRevertReason(revertData);
assertEq(revertReason, "Referral code too long", "Unexpected revert reason for long referral code");
}
}
​
function testValidReferralCode() public {
// Set up a valid referral code
bytes memory validReferralCode = "ValidCode";
​
// Doing tests
(bool success, bytes memory revertData) = address(tradingAccountBranch).call(
abi.encodeWithSignature("createTradingAccount(bytes,bool)", validReferralCode, true)
);
​
if (success) {
// The test is successful if there are no reverts
assertTrue(success, "Expected createTradingAccount to succeed with valid referral code");
} else {
// Check the reason for revert if failure occurs
string memory revertReason = getRevertReason(revertData);
revert(string(abi.encodePacked("Unexpected revert reason: ", revertReason)));
}
}
​
// Helper function to get the reason for revert
function getRevertReason(bytes memory revertData) internal pure returns (string memory) {
if (revertData.length == 0) return "Unknown"; // No data
if (revertData.length < 68) return "Unknown"; // Data is too short
​
// Decode the revert reason
bytes memory reason = new bytes(revertData.length - 4);
for (uint i = 0; i < reason.length; i++) {
reason[i] = revertData[i + 4];
}
​
// Try decoding as string
return string(reason);
}
}

forge test --match-path test/TradingAccountBranchTest.t.sol
[⠒] Compiling...
[⠆] Compiling 1 files with Solc 0.8.25
[⠰] Solc 0.8.25 finished in 2.06s
Compiler run successful!

Ran 2 tests for test/TradingAccountBranchTest.t.sol:TradingAccountBranchTest
[FAIL. Reason: Unexpected revert reason for long referral code: Unknown != Referral code too long] testReferralCodeLength() (gas: 81009)
[FAIL. Reason: revert: Unexpected revert reason: Unknown] testValidReferralCode() (gas: 77494)
Suite result: FAILED. 0 passed; 2 failed; 0 skipped; finished in 2.28ms (630.40µs CPU time)

Ran 1 test suite in 22.95ms (2.28ms CPU time): 0 tests passed, 2 failed, 0 skipped (2 total tests)

Failing tests:
Encountered 2 failing tests in test/TradingAccountBranchTest.t.sol:TradingAccountBranchTest
[FAIL. Reason: Unexpected revert reason for long referral code: Unknown != Referral code too long] testReferralCodeLength() (gas: 81009)
[FAIL. Reason: revert: Unexpected revert reason: Unknown] testValidReferralCode() (gas: 77494)

Encountered a total of 2 failing tests, 0 tests succeeded

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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