Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolFactory.sol#L63

Vulnerability details

Impact

This If statement

if (pair != address(0)) Error.zeroAddress();

says that if pair is not zero address it should throw an error. so according to it, "pair" must be a zero address.
And the error thrown is even meant for zero address, "Error.zeroAddress()" which means there's a mistake in the if statement.

Proof of Concept

According to these lines of code

if (optionPair == address(0)) Error.zeroAddress();
pair = pairs[optionPair];

which comes before the wrong if statement, pair isn't a zero address and it's not supposed to be a zero address. This external function create() will always throw the error, "Error.zeroAddress".

Tools Used

Manual Review, Remix

Recommended Mitigation Steps

correct the If statement to

if (pair == address(0)) Error.zeroAddress();

Agreements

If you are a C4 Certified Contributor by commenting or interacting with this repo prior to public release of the contest report, you agree that you have read the Certified Warden docs and agree to be bound by:

• C4 Certified Contributor Terms and Conditions,
• C4 Code of Professional Conduct, and
• the disclosure guidelines below.

To signal your agreement to these terms, add a 👍 emoji to this issue.

Code4rena staff reserves the right to disqualify anyone from this role and similar future opportunities who is unable to participate within the above guidelines.

Disclosures

Sponsors may elect to add team members and contractors to assist in sponsor review and triage. All sponsor representatives added to the repo should comment on this issue to identify themselves.

To ensure contest integrity, the following potential conflicts of interest should also be disclosed with a comment in this issue:

1. any sponsor staff or sponsor contractors who are also participating as wardens
2. any wardens hired to assist with sponsor review (and thus presenting sponsor viewpoint on findings)
3. any wardens who have a relationship with a judge that would typically fall in the category of potential conflict of interest (family, employer, business partner, etc)
4. any other case where someone might reasonably infer a possible conflict of interest.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L176

Vulnerability details

Impact

This if statement -

if (maturity < block.TimeStamp(0)) Error.alreadyMatured(maturity, blockTimestamp(0)); 

will always be false, because blockTimeStamp is initialised to zero.
And as maturity is a uint, 0 is it's least figure. uint can never be < 0; so the condition of the if statement will never be met.

Proof of Concept

So let's say maturity is "1" for instance being a uint.
1 < 0 will always be false. so the check for maturity, i.e to know if it's already matured, is invalid.

Tools Used

Manual Review

Recommended Mitigation Steps

don't initialise block.TimeStamp to 0

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L52
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L66-L69
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L71-L73

Vulnerability details

Impact

The problem with this vulnerability is that it will allow an attacker to repeatedly call the guarded function by bypassing the reentrancy guard. The contract uses a mapping called reentrancyGuards to track if a given msg.sender has passed the guard and prevent reentrancy attacks. However, it doesn't check if the msg.sender has already passed the guard, meaning an attacker could repeatedly call the guarded function by calling it from multiple contracts or multiple addresses controlled by the attacker. It can be exploited to cause a DoS attack, consume all the gas, or disrupt the normal function of the contract.

Proof of Concept

One way an attacker could exploit this vulnerability would be to create a malicious contract that repeatedly calls the vulnerable function before the reentrancy guard has been reset, allowing the attacker to repeatedly execute the function and potentially steal assets from the contract.

Code.

contract Attacker {
    address victim;
    function attack() public {
        victim.call(bytes4(keccak256("vulnerableFunction()")));
        victim.call(bytes4(keccak256("vulnerableFunction()")));
        victim.call(bytes4(keccak256("vulnerableFunction()")));
    }
}

attack function of the Attacker contract repeatedly calls the vulnerableFunction() function of the victim contract before the reentrancy guard has been reset, potentially allowing the attacker to exploit a reentrancy vulnerability.

Tools Used

Manual audit, Vs Code.

Recommended Mitigation Steps

The contract should include a check that verifies if the msg.sender has already passed the reentrancy guard before allowing the function to execute, using a mapping to track the state of the reentrancy guard for each msg.sender and checking its status before allowing the function to execute.

Code.

// Mapping to track reentrancy guard status for each address
mapping(address => bool) public reentrancyGuards;

function protectedFunction() public {
    require(!reentrancyGuards[msg.sender], "Reentrancy attack detected.");
    reentrancyGuards[msg.sender] = true;
    // Function logic
    reentrancyGuards[msg.sender] = false;
}

Another option is to add a check in the raiseGuard function to see if the msg.sender has already passed the guard for a specific strike and maturity by adding a mapping of address to bool, where the key is the address of the msg.sender and the value is a boolean that indicates whether the msg.sender has already passed the guard for a specific strike and maturity.

Code.

mapping (address => mapping (bytes32 => mapping (bytes32 => bool))) private reentrancyGuards;

function raiseGuard(bytes32 strike, bytes32 maturity) private {
    require(!reentrancyGuards[msg.sender][strike][maturity], "Reentrancy guard triggered.");
    reentrancyGuards[msg.sender][strike][maturity] = true;
}

This uses a nested mapping structure to store the reentrancy guards, where the outermost mapping is for the address of the msg.sender, the second mapping is for the strike, and the innermost mapping is for the maturity.

Or by using a nonce system where the contract keeps track of the number of times a user has called the contract, if the same nonce is provided twice, the contract will know that it is a reentrancy attempt.

Code.

mapping(address => uint256) private nonce;

function raiseGuard(bytes32 strike, bytes32 maturity) private {
    require(nonce[msg.sender] == msg.value, "Reentrancy guard triggered.");
    nonce[msg.sender] = msg.value + 1;
}

Using a mapping where the key is the address of the msg.sender and the value is the nonce. The function first checks if the nonce provided in msg.value matches the nonce stored in the mapping for the msg.sender. If the nonces match, the function increments the nonce stored in the mapping by 1 to indicate that the msg.sender has passed the guard and allows the function to proceed. If the nonces do not match, the function throws an exception to indicate that reentrancy has been detected.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2PoolFactory.sol#L65
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25

Vulnerability details

Impact

The create() function calls the deploy() function to deploy smart contracts.
However, the function does not verify the result of the deploy() function in order to make sure that it was successful.
This could lead to an unexpected behavior by the function.

Proof of Concept

The deploy() function is called without checking its result:

pair = deploy(address(this), optionPair, transactionFee, protocolFee);
pairs[optionPair] = pair;
emit Create(msg.sender, optionPair, pair);

Recommended Mitigation Steps

Verifiy if the deploy() function worked as expected in order to revert if there is an issue.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-library/lib/forge-std/src/StdCheats.sol#L465
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-library/lib/forge-std/src/StdCheats.sol#L469
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-option/src/TimeswapV2Option.sol#L71
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/lib/forge-std/src/StdCheats.sol#L458
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/lib/forge-std/src/StdCheats.sol#L462
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2Pool.sol#L83
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/lib/forge-std/src/StdCheats.sol#L465
https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/lib/forge-std/src/StdCheats.sol#L469

Vulnerability details

Impact

The vulnerability of block.timestamp in smart contracts is related to the fact that the timestamp of a block is provided by the miner who mined the block. As a result, the timestamp is not guaranteed to be accurate or to be the same across different nodes in the network. In particular, an attacker can potentially mine a block with a timestamp that is favorable to them, known as "selective packing".

For example, an attacker could mine a block with a timestamp that is slightly in the future, allowing them to bypass a time-based restriction in a smart contract that relies on block.timestamp. This could potentially allow the attacker to execute a malicious action that would otherwise be blocked by the restriction.

Proof of Concept

• packages/v2-library/lib/forge-std/src/StdCheats.sol#L465 => vm.warp(block.timestamp + time);
• packages/v2-library/lib/forge-std/src/StdCheats.sol#L469 => vm.warp(block.timestamp - time);
• packages/v2-option/src/TimeswapV2Option.sol#L71 => return uint96(block.timestamp); // truncation is desired
• packages/v2-pool/lib/forge-std/src/StdCheats.sol#L458 => vm.warp(block.timestamp + time);
• packages/v2-pool/lib/forge-std/src/StdCheats.sol#L462 => vm.warp(block.timestamp - time);
• packages/v2-pool/src/TimeswapV2Pool.sol#L83 => return uint96(block.timestamp + durationForward); // truncation is desired
• packages/v2-token/lib/forge-std/src/StdCheats.sol#L465 => vm.warp(block.timestamp + time);
• packages/v2-token/lib/forge-std/src/StdCheats.sol#L469 => vm.warp(block.timestamp - time);

Tools Used

• Private self-made tool for static analysis
• Manual Review, Remix IDE

Recommended Mitigation Steps

Developers should avoid using block.timestamp in their smart contracts and instead use an alternative timestamp source, such as an oracle, that is not susceptible to manipulation by a miner.

References:

• Timestamp dependence | Solidity Best Practices for Smart Contract Security
• What Is Timestamp Dependence?

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2Pool.sol#L277

Vulnerability details

Impact

The reentrancy protection on TimeswapV2Pool of mint, burn, leverage and deleverage can reentrant and attacker may mint more than what he was supposed to mint

Proof of Concept

The TimeswapV2Pool. deleverage function (taking that function as example for the report) works that way,
it first raises the reentrancy guard on the provided param.strike and param.maturity

raiseGuard(param.strike, param.maturity);

then it made the deleveragation call :

               (long0Amount, long1Amount, shortAmount, data) = pools[param.strike][param.maturity].deleverage(param, transactionFee, protocolFee, blockTimestamp(durationForward));


        ITimeswapV2Option(optionPair).transferPosition(param.strike, param.maturity, param.to, TimeswapV2OptionPosition.Short, shortAmount);


        ...etc

after that it makes a call to msg.sender to transfer the position to address(this) :

 data = ITimeswapV2PoolDeleverageCallback(msg.sender).timeswapV2PoolDeleverageCallback(
            TimeswapV2PoolDeleverageCallbackParam({strike: param.strike, maturity: param.maturity, long0Amount: long0Amount, long1Amount: long1Amount, shortAmount: shortAmount, data: data})
        );

once done the reentrancy guard will lower reentrancy of the provided param.strike and param.maturity,
Attacker can make a reentrancy using the following scenario :

1. Calls TimeswapV2Pool. deleverage with param.strike X and param.maturity X
2. the function will makes the positionsOf calls to optionPair, then it will calls to msg.sender to transfer the positions into this address.
3. as mentioned in the comment // Ask the msg.sender to transfer the positions into this address (line 402)
4. the msg.sender is contract, and the fallback calls the TimeswapV2Pool.deleverage (or even other functions as such TimeswapV2Pool.mint) with param.strike Y and param.maturity Y
5. the transfer from msg.sender to address(this) (wasn't completed yet) the msg.sender makes a second call to get positions
6. The reaiseGuard will not protect since it was raised for param.strike X and param.maturity X not param.strike Y and param.maturity Y
7. and the call will continues .. within the fallbakc the attacker makes call to TimeswapV2Pool.deleverage with param.strike Z and param.maturity Z
8. ... and so on

Tools :

Manual

Recommended Mitigation Steps

uses the simple lock based reentrancy guard to lock all the TimeswapV2Pool calls

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolFactory.sol#L59-L70
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25-L31

Vulnerability details

Impact

According to the whitepaper only liquidity providers should be able to create pools.
but here a lender or borrower can create a pool as a liquidity provider and manipulate lending positions/borrowing positions to be in their favor.
And also pay transaction fees to themselves.

Proof of Concept

There's no check to make sure only Liquidity Providers can create pools as stated in the whitepaper.

Tools Used

Manual Review

Recommended Mitigation Steps

add a form of protection to ensure that both internal function deploy() in TimeswapV2PoolDeployer.sol and external function create() in TimeswapV2PoolFactory.sol which calls the internal function deploy(), can be used by only Liquidity providers, i.e make a check to ensure that anyone who creates a pool as a Liquidity Provider won't be able to be a lender or borrower anymore.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2Pool.sol#L83

Vulnerability details

Impact

functions may reverts and make the contract fails to work

Proof of Concept

The burn, deleverage, leverage functions on TimeswapV2Pool contract uses the internal blockTimestamp function, the function sums the current block.timestamp with the given durationForward integer, the sum however assumes that all will be a 96 bits number,

return uint96(block.timestamp + durationForward); // truncation is desired

if one of the burn, deleverage, leverage functions uses the durationForward as huge number ,high enough as durationForward - block.timestamp > 79228162514264337593543950335, it will make the function reverts and fails to works

Tools Used

Manual

Recommended Mitigation Steps

it's better to use uint256, uint232, uint216

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L277-L287

Vulnerability details

Impact

mint function in the smart contract uses a callback function, ITimeswapV2PoolMintCallback(msg.sender).timeswapV2PoolMintCallback(), to ask the msg.sender to transfer positions into the contract address. However, it does not check if the callback function is implemented by the msg.sender contract. If the msg.sender contract does not implement the callback, the function will throw an exception.

This will cause the mint function to fail, preventing users from depositing positions into the pool and minting liquidity. This can also cause the smart contract to enter an error state and stop functioning properly.

Furthermore, an attacker can exploit this vulnerability by creating a malicious contract that doesn't implement the callback function and calling the mint function. This can cause the smart contract to fail, preventing legitimate users from using the mint function, and can cause a loss of funds.

Proof of Concept

Attacker can exploit this vulnerability by creating a malicious contract that does not implement the required callback function ITimeswapV2PoolMintCallback.timeswapV2PoolMintCallback() and then calling the mint function of the smart contract while sending transactions from the malicious contract.

Code.

contract Malicious {
    function attack() external {
        // call the "mint" function of the target contract
        // without implementing the required callback function
    }
}

When the mint function is called, it will call the callback function on the msg.sender contract, but since the malicious contract does not implement the callback function, the function will throw an exception and the smart contract will enter an error state.

This can cause the mint function to fail, preventing legitimate users from depositing positions into the pool and minting liquidity. It can also cause the smart contract to stop functioning properly, leading to a potential loss of funds.

Tools Used

Manual audit, Vs Code

Recommended Mitigation Steps

Check if the msg.sender contract implements the required callback function before calling it. This can be done by using the delegatecall or callcode function to check the implementation of the callback function in the msg.sender contract.

Code.

function mint(
        TimeswapV2PoolMintParam calldata param,
        bool isQuote,
        uint96 durationForward
    ) private noDelegateCall returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {
    ParamLibrary.check(param, blockTimestamp(durationForward));
    raiseGuard(param.strike, param.maturity);

    // Calculate the main logic of mint function.
    (liquidityAmount, long0Amount, long1Amount, shortAmount, data) = pools[param.strike][param.maturity].mint(param, blockTimestamp(durationForward));

    // Check if the msg.sender contract implements the required callback function
    assembly {
        if iszero(extcodesize(msg.sender)) {
            revert(0, 0)
        }
    }

    // Calculate the amount of long0 position, long1 position, and short position required by the pool.

    // long0Amount chosen could be zero. Skip the calculation for gas efficiency.
    uint256 long0BalanceTarget;
    if (long0Amount != 0) long0BalanceTarget = ITimeswapV2Option(optionPair).positionOf(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Long0) + long0Amount;

    // long1Amount chosen could be zero. Skip the calculation for gas efficiency.
    uint256 long1BalanceTarget;
    if (long1Amount != 0) long1BalanceTarget = ITimeswapV2Option(optionPair).positionOf(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Long1) + long1Amount;

    // shortAmount cannot be zero.
    uint256 shortBalanceTarget = ITimeswapV2Option(optionPair).positionOf(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Short);

    // Check if msg.sender contract has the callback function before calling it
    assembly {
        let size := extcodesize(msg.sender)
        require(size > 0, "Callback function not implemented by msg.sender contract")
    }

    // Ask the msg.sender to transfer the positions into this address.
    data = ITimeswapV2PoolMintCallback(msg.sender).timeswapV2PoolMintCallback(
        TimeswapV2PoolMintCallbackParam({
            strike: param.strike,
            maturity: param.maturity,
            long0Amount: long0Amount,
            long1Amount: long1Amount,
            shortAmount: shortAmount,
            liquidityAmount: liquidityAmount,
            isQuote: isQuote,
            data: data
        })
    );

    // Transfer the positions
    if (long0Amount != 0) ITimeswapV2Option(optionPair).transferPosition(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Long0, long0Amount);

    if (long1Amount != 0) ITimeswapV2Option(optionPair).transferPosition(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Long1, long1Amount);

    ITimeswapV2Option(optionPair).transferPosition(param.strike, param.maturity, address(this), TimeswapV2OptionPosition.Short, shortAmount);

    // Add liquidity
    liquidityAmount = liquidityAmount.add(transactionFee).add(protocolFee);
    address(this).transfer(liquidityAmount);
}

This reform uses the extcodesize assembly function to check if the msg.sender contract has any code deployed at its address. If it does not have any code deployed, the function reverts with an error message. This check ensures that the callback function is implemented by the msg.sender contract before calling it and that the function will not throw an exception when the callback is not implemented.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L231-L237

Vulnerability details

Impact

collect function does not check if the msg.sender has the authority to collect positions from the pool, it should check if msg.sender is the owner of the positions.

function collect(uint256 strike, uint256 maturity, address long0To, address long1To, address shortTo, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount) private {
    if (long0Amount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, long0To, TimeswapV2OptionPosition.Long0, long0Amount);

    if (long1Amount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, long1To, TimeswapV2OptionPosition.Long1, long1Amount);

    if (shortAmount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, shortTo, TimeswapV2OptionPosition.Short, shortAmount);
}

The function does not check if the msg.sender has the authority to collect positions from the pool, it directly calls the transferPosition function on the ITimeswapV2Option contract. It should check if the msg.sender is the owner of the positions by calling the require(msg.sender == owner) before calling the transferPosition function.

Proof of Concept

For exploiting this vulnerability would involve.

The attacker would need to find a pool that has some positions that the attacker does not own. The attacker can use the `ITimeswapV2Option.positionOf()` function to check the ownership of positions in a pool.

The attacker would then call the `collect()` function on the `TimeswapV2Pool` contract, passing in the strike, maturity, and the address they want to transfer the positions `to`. The attacker can use the `long0Amount`, `long1Amount`, and `shortAmount` parameters to specify the number of positions they want to transfer.

Code.

function exploit(address pool, uint256 strike, uint256 maturity, address long0To, address long1To, address shortTo, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount) public {
    ITimeswapV2Pool(pool).collect(strike, maturity, long0To, long1To, shortTo, long0Amount, long1Amount, shortAmount);
}

Since the function does not check for authorization, the positions will be transferred to the specified address without any checks. The attacker can then use these positions for their own gain, and steal assets from the legitimate owners while the legitimate owner of the positions will have lost them.

Tools Used

Manual audit, vs code

Recommended Mitigation Steps

Add an authorization check in the collect() function to ensure that the msg.sender is the owner of the positions before allowing the transfer.

Code.

function collect(uint256 strike, uint256 maturity, address long0To, address long1To, address shortTo, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount) private {
    address long0Owner = ITimeswapV2Option(optionPair).ownerOf(strike, maturity, TimeswapV2OptionPosition.Long0);
    address long1Owner = ITimeswapV2Option(optionPair).ownerOf(strike, maturity, TimeswapV2OptionPosition.Long1);
    address shortOwner = ITimeswapV2Option(optionPair).ownerOf(strike, maturity, TimeswapV2OptionPosition.Short);
    require(msg.sender == long0Owner || msg.sender == long1Owner || msg.sender == shortOwner, "Unauthorized: msg.sender is not the owner of the positions");
    if (long0Amount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, long0To, TimeswapV2OptionPosition.Long0, long0Amount);

    if (long1Amount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, long1To, TimeswapV2OptionPosition.Long1, long1Amount);

    if (shortAmount != 0) ITimeswapV2Option(optionPair).transferPosition(strike, maturity, shortTo, TimeswapV2OptionPosition.Short, shortAmount);
}

This will check if the msg.sender is the owner of the positions before allowing the transfer, by checking the return value of the ITimeswapV2Option.ownerOf() function for each position. The require() function will ensure that the check passes and will revert the transaction if the msg.sender is not the owner of the positions.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L46
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L53
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L61

Vulnerability details

Impact

bytes32 key = keccak256(abi.encode(token0, token1, strike, maturity));

In the raiseGuard() function and the lowerGuard() function.

function raiseGuard(address token0, address token1, uint256 strike, uint256 maturity) private {
        bytes32 key = keccak256(abi.encode(token0, token1, strike, maturity));

        reentrancyGuards[key].check();
        reentrancyGuards[key] = ReentrancyGuard.ENTERED;
    }

function lowerGuard(address token0, address token1, uint256 strike, uint256 maturity) private {
        bytes32 key = keccak256(abi.encode(token0, token1, strike, maturity));
        reentrancyGuards[key] = ReentrancyGuard.NOT_ENTERED;
    }

This vulnerability could allow an attacker to bypass the reentrancy guard and potentially exploit the contract in unexpected ways beyond imagination, is caused by the use of the keccak256 function to generate the reentrancyGuards key. The key is generated by taking the abi-encoded values of token0, token1, strike, and maturity and passing them through the keccak256 function. However, since keccak256 is a deterministic function, it is possible for an attacker to brute force the key and call the function again.

The key is used to track the state of the reentrancy guard for the contract and is used in the raiseGuard() and lowerGuard() functions. If an attacker is able to brute force the key, they can call the function again, potentially allowing them to bypass the reentrancy guard and call other functions in the contract multiple times before the guard is raised again.

Proof of Concept

Exploit Code.

// Assume that the attacker has the addresses of token0, token1, and the TimeswapV2Token contract
address token0 = 0x123456789abcdef01234567890abcdef01234567;
address token1 = 0xabcdef01234567890abcdef01234567890abcdef;
address timeswapV2Token = 0xdef01234567890abcdef01234567890abcdef012;

// The attacker can use a for loop to brute force the key by iterating through all possible values of strike and maturity
for (uint256 strike = 0; strike < 10**18; strike++) {
    for (uint256 maturity = 0; maturity < 10**18; maturity++) {
        bytes32 key = keccak256(abi.encode(token0, token1, strike, maturity));

        // If the key is found, the attacker can call the function again, potentially bypassing the reentrancy guard
        if (timeswapV2Token.call(key)) {
            // Attacker can now call other functions in the contract multiple times before the guard is raised again
            timeswapV2Token.transfer(msg.sender, someAddress, someAmount); //...
    }
  }
}

In this example, the attacker uses a nested for loop to iterate through all possible values of strike and maturity, generating the key by passing the abi-encoded values of token0, token1, strike, and maturity through the keccak256 function.

Once the key is found, the attacker then calls the function again, potentially bypassing the reentrancy guard. With the reentrancy guard bypassed, the attacker can call other functions in the contract multiple times before the guard is raised again, potentially allowing them to exploit the contract in unexpected ways, such as by repeatedly calling functions that transfer funds or tokens, or by repeatedly calling functions that change the state of the contract.

Tools Used

Manual audit, Vs Code, Remix.

Recommended Mitigation Steps

Instead of using the keccak256 function to generate the key, consider using a secure random number generator or using the address of the contract as a seed.

Code.

function generateRandomKey() private pure returns(bytes32){
   return keccak256(abi.encodePacked(address(this),block.timestamp,block.blockhash(block.number-1),uint256(uint256(uint256(keccak256(abi.encodePacked(address(this),block.timestamp,block.blockhash(block.number-1)))) * uint256(keccak256(abi.encodePacked(address(this),block.timestamp,block.blockhash(block.number-1)))))));
}

Implement a limit for the number of function calls that can be made before the reentrancy guard is raised again.

Code.

mapping(bytes32 => uint256) private reentrancyCalls;

function lowerGuard(address token0, address token1, uint256 strike, uint256 maturity) private {
    bytes32 key = generateRandomKey();
    if(reentrancyCalls[key] < 10){
        reentrancyGuards[key] = ReentrancyGuard.NOT_ENTERED;
        reentrancyCalls[key] = 0;
    }else{
        reentrancyCalls[key] ++;
    }
}

We are keeping track of the number of times a function is called with a specific key, and once the number of calls reaches 10, the reentrancy guard is raised again, and the number of calls is reset to 0. This will prevent an attacker from repeatedly calling the function and bypassing the reentrancy guard.

It's important to use a circuit breaker can be implemented to pause the contract's functionality in case of an emergency or unexpected behavior.

Code.

bool public contractPaused = false;

modifier onlyWhenNotPaused() {
    require(!contractPaused);
    _;
}

function pause() public onlyOwner {
    contractPaused = true;
}

function unpause() public onlyOwner {
    contractPaused = false;
}

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L66-L68

Vulnerability details

Impact

function positionOf(address owner, TimeswapV2TokenPosition calldata timeswapV2TokenPosition) public view returns (uint256 amount) {
    amount = ERC1155.balanceOf(owner, _timeswapV2TokenPositionIds[timeswapV2TokenPosition]);
}

positionOf() function in the contract is used to retrieve the amount of a certain position that an owner has. However, the logic behind how positions are assigned and transferred is not clearly defined in the contract. This can make it difficult to understand how the contract is supposed to function and may lead to unexpected behavior.

Proof of Concept

An attacker could potentially exploit the lack of clear logic in the positionOf() function by manipulating the input passed to the function in order to retrieve positions that they should not have access to. For example, if the attacker is able to figure out how the positions are assigned, they could potentially call the positionOf() function with an input that would allow them to access positions that they should not have access to.

Tools Used

Manual review, Vs Code.

Recommended Mitigation Steps

Defined and implemented the positionOf() function to ensure that positions are assigned and transferred correctly. Additionally, proper validation and access control should be implemented in the positionOf() function to prevent unauthorized access to positions.

Code.

function positionOf(address owner, TimeswapV2TokenPosition calldata timeswapV2TokenPosition) public view returns (uint256 amount) {
    require(msg.sender == owner, "Unauthorized access to positions");
    require(_timeswapV2TokenPositions[owner] == timeswapV2TokenPosition, "Invalid position for this owner");
    amount = ERC1155.balanceOf(owner, _timeswapV2TokenPositionIds[timeswapV2TokenPosition]);
}

The function checks that the msg.sender is the same as the provided owner address, and that the provided position belongs to the owner. If these conditions are not met, the function will not execute and will return an error message.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25-L31

Vulnerability details

Impact

function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal returns (address poolPair) {
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});

        poolPair = address(new TimeswapV2Pool{salt: keccak256(abi.encode(optionPair))}());

        delete parameter;
    }

deploy function does not have any protection against reentrancy attacks, which means a "malicious" contract could call the deploy function multiple times before it has a chance to delete the parameter struct. This vulnerability can allow an attacker to repeatedly call the deploy function and deploy multiple contracts, leading to unexpected behavior and loss of funds.

Proof of Concept

An attacker can create a malicious contract that calls the deploy function of the TimeswapV2PoolDeployer contract repeatedly before it has a chance to delete the parameter struct.

Code.

pragma solidity = 0.8.8;

contract Malicious {
    TimeswapV2PoolDeployer public timeswapV2PoolDeployer;

    constructor(address _timeswapV2PoolDeployer) public {
        timeswapV2PoolDeployer = TimeswapV2PoolDeployer(_timeswapV2PoolDeployer);
    }

    function exploit() public {
        while (true) {
            timeswapV2PoolDeployer.deploy(address(0x0), address(0x0), 0, 0);
        }
    }
}

The Malicious contract takes in the address of the TimeswapV2PoolDeployer contract during the constructor and assigns it to the public variable timeswapV2PoolDeployer. The exploit function is then called, which repeatedly calls the deploy function of the TimeswapV2PoolDeployer contract before it has a chance to delete the parameter struct, causing multiple instances of the TimeswapV2Pool contract to be deployed.

Since there is no protection against reentrancy attacks in the deploy function of the TimeswapV2PoolDeployer contract, the malicious contract can repeatedly call the deploy function, potentially leading to unexpected behavior and loss of funds.

Tools Used

Manual audit, Remix, vs Code

Recommended Mitigation Steps

It's crucial to add a reentrancy guard to the deploy function, using Mutex pattern is a way to prevent reentrancy by using aboolean` variable to indicate if the contract is currently executing a sensitive function.

Code.

bool public mutex;

function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal returns (address poolPair) {
        require(!mutex, "Reentrancy detected");
        mutex = true;
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});
        poolPair = address(new TimeswapV2Pool{salt: keccak256(abi.encode(optionPair))}());
        delete parameter;
        mutex = false;
    }

There are several libraries available that provide reentrancy protection, such as OpenZeppelin's Ownable.sol which provides a modifier onlyOwner that prevents reentrancy by checking if the msg.sender is the contract's owner.

Code.

import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/access/Ownable.sol";

contract TimeswapV2PoolDeployer is Ownable, ITimeswapV2PoolDeployer {
    function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal onlyOwner returns (address poolPair) {
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocol

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L245-L250
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L252-L257

Vulnerability details

Impact

function mint(
    TimeswapV2PoolMintParam calldata param,
    uint96 durationForward
) external override returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {
    return mint(param, true, durationForward);
}

and

function mint(
    TimeswapV2PoolMintParam calldata param,
    bool isQuote,
    uint96 durationForward
) private noDelegateCall returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {
    ParamLibrary.check(param, blockTimestamp(durationForward));

The durationForward parameter is used to forward the timestamp of the block, and it could be potentially used for a timestamp manipulation attack.

mint function in the smart contract has an optional parameter durationForward which is used to forward the timestamp of the block. This feature could be potentially used for a timestamp manipulation attack. An attacker could manipulate the timestamp of the block to execute a function before or after a specific time, which could allow them to bypass certain time-based restrictions or access unauthorized data.

Proof of Concept

The attacker could exploit this vulnerability by manipulating the durationForward parameter in the mint function. For example, an attacker could call the mint function with a large durationForward value, which would forward the timestamp of the block to a time in the future.

Let's assume that the smart contract has a time lock feature that prevents the mint function from being executed until a certain date. By manipulating the timestamp, the attacker could bypass the lock and execute the function before the specified date.

Code.

contract Attacker {
    address _timeswapV2Pool;
    constructor(address timeswapV2Pool) public {
        _timeswapV2Pool = timeswapV2Pool;
    }

    function executeAttack() public {
        // Attacker calls the mint function with a large durationForward value
        // to forward the timestamp of the block to a time in the future, bypassing the time lock
        _timeswapV2Pool.mint(param, true, 1000000000);
    }
}

In this scenario, the attacker creates an instance of the Attacker contract, passing the address of the TimeswapV2Pool contract as a parameter. Then they call the executeAttack() function which calls the mint function on the TimeswapV2Pool contract with a large durationForward value (1000000000), which would forward the timestamp of the block to a time in the future.

Also, blockTimestamp function, is not a reliable source of time. An attacker can manipulate the timestamp of the blocks and therefore execute the function at a different time than the one intended by the smart contract.

Tools Used

Manual audit, Vs Code, Remix IDE

Recommended Mitigation Steps

Remove the durationForward parameter from the "mint" function, and instead use the actual block timestamp in the function. This will prevent any potential manipulation of the timestamp.

add a guard that checks if the durationForward value is within a valid range. For example, you can check that the value is not greater than the current "block's timestamp + some constant value".

Code.

function mint(
    TimeswapV2PoolMintParam calldata param,
    bool isQuote,
    uint96 durationForward
) private noDelegateCall returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {
    require(block.timestamp + MAX_FORWARD_DURATION > durationForward, "Duration forward is too high");
    ParamLibrary.check(param, blockTimestamp(durationForward));
    raiseGuard(param.strike, param.maturity);

    // Rest of the function
}

By adding such a guard, the function will only allow to use the durationForward if it's within the valid range and will prevent the attacker from manipulating the timestamp for their advantage.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L175

Vulnerability details

Impact

TimeswapV2Pool contract, the initialize function was missing access controls, allowing any user to initialize the created pool contract. By front-running , the incorrect rate parameter may be supplied to the created pool contract.

Proof of Concept

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L175

Tools Used

manually

Recommended Mitigation Steps

add access controls to this function.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolFactory.sol#L53

Vulnerability details

Impact

when numberOfPairs function is called, it will always 0.

Proof of Concept

when number of pairs function called it will get always 0 because it works with getbyIndex array's length but this array has never been updated in any other function in the contract.

Tools Used

Recommended Mitigation Steps

Array can be updated when create function called or just a uint value can be used for the total number of pairs. Which added one when create function called and is returned when numberOfPairs function called.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L34
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2Token.sol#L41-L43

Vulnerability details

Impact

address public immutable optionFactory;

It is in the contract constructor where it is initialized with a chosen option factory.

constructor(address chosenOptionFactory) ERC1155("Timeswap V2 address") {
        optionFactory = chosenOptionFactory;
}

optionFactory variable is declared as address public immutable, which means that once it is set in the contract constructor, it cannot be changed.

It could be problematic for several reasons. When there are bugs or vulnerabilities discovered in the option factory contract, it may be necessary to replace it with a new version. However, if the optionFactory address is immutable, this would not be possible. Additionally, if the option factory contract is controlled by a malicious actor, this could lead to serious security risks.

Proof of Concept

An attacker could exploit this vulnerability by creating a malicious version of the option factory contract and then finding a way to set the optionFactory address to point to their malicious contract.

If a malicious actor is able to change the value of the optionFactory address, they could use the following.

1. Creating a malicious version of the option factory contract that has some malicious functionality.

2. Finding a vulnerability in the TimeswapV2Token contract that allows changing the value of the optionFactory address.

3. Exploiting this vulnerability to set the optionFactory address to point to the malicious contract.

Code.

address maliciousOptionFactory = address(maliciousOptionFactoryContract);
TimeswapV2Token timeswapV2Token = TimeswapV2Token(address(contract));
timeswapV2Token.changeOptionFactoryAddress(maliciousOptionFactory);

Once the attacker has successfully set the optionFactory address to point to their malicious contract, they could use the malicious functionality to steal funds, manipulate data, or perform other malicious actions.

Tools Used

Manual audit, Vs Code

Recommended Mitigation Steps

Remove the immutable keyword from the optionFactory variable declaration and make sure that the address can be changed when necessary.

address public optionFactory;

Add a function to change the option factory address to the contract that could be called by the owner or a trusted address.

function changeOptionFactoryAddress(address newOptionFactory) public onlyOwner {
    optionFactory = newOptionFactory;
}

Implement a circuit breaker that allows pausing the contract and updating the option factory address.

bool public contractPaused = false;

modifier onlyWhenUnpaused {
    require(!contractPaused);
    _;
}

function pause() public onlyOwner {
    contractPaused = true;
}

function unpause() public onlyOwner {
    contractPaused = false;
}

function changeOptionFactoryAddress(address newOptionFactory) public onlyOwner onlyWhenUnpaused {
    optionFactory = newOptionFactory;
}

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25-L26
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L26

Vulnerability details

Impact

function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal returns (address poolPair) {
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});

parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});

Here the input parameters are assigned without any validation, allowing any address to be passed as the poolFactory address, even if it's not a valid contract address.

It allows a malicious attacker to pass an arbitrary address as the poolFactory address, potentially leading to unexpected behavior and loss of funds. An attacker can pass their own address as the poolFactory address, allowing them to gain control of the deployed contract and steal funds from the deployed contract.

Proof of Concept

We'll start by creating a malicious contract that calls the deploy function of the TimeswapV2PoolDeployer contract with an arbitrary address as the poolFactory parameter.

Code.

pragma solidity = 0.8.8;

contract Malicious {
    TimeswapV2PoolDeployer public timeswapV2PoolDeployer;

    constructor(address _timeswapV2PoolDeployer) public {
        timeswapV2PoolDeployer = TimeswapV2PoolDeployer(_timeswapV2PoolDeployer);
    }

    function exploit() public {
        address maliciousAddress = address(this);
        timeswapV2PoolDeployer.deploy(maliciousAddress, address(0x0), 0, 0);
    }
}

The Malicious contract takes in the address of the TimeswapV2PoolDeployer contract during the constructor and assigns it to the public variable timeswapV2PoolDeployer. The exploit function is then called, which calls the "deploy" function of the TimeswapV2PoolDeployer contract, passing in the address of the malicious contract as the poolFactory parameter.

Since there is no validation of the input parameters in the deploy function of the TimeswapV2PoolDeployer contract, the malicious contract can pass in any address as the poolFactory parameter, including its own address. As a result, the deployed contract will be controlled by the malicious contract, allowing the attacker to access and potentially steal any funds stored within the deployed contract.

Tools Used

Manual audit, Vs Code, Remix IDE.

Recommended Mitigation Steps

Check the code at the address. The poolFactory address can be passed through an external library that checks if the code at the address is a contract or not. This can be done by calling the code function of the Ethereum Virtual Machine (EVM) on the address and checking if it returns a non-zero value. Example of how this can be done in the deploy function.

Code.

function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal returns (address poolPair) {
        require(keccak256(abi.encodePacked(address(this).balance)) != keccak256(abi.encodePacked(address(poolFactory).balance)), "Invalid poolFactory address");
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});

There are several libraries and packages available that can check if an address is a contract or not. These libraries usually check the code at the address and return a boolean value indicating if the address is a contract or not. Here is an example of how this can be done in the "deploy" function.

Code.

import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/utils/Address.sol";

function deploy(address poolFactory, address optionPair, uint256 transactionFee, uint256 protocolFee) internal returns (address poolPair) {
        require(Address.isContract(poolFactory), "Invalid poolFactory address");
        parameter = Parameter({poolFactory: poolFactory, optionPair: optionPair, transactionFee: transactionFee, protocolFee: protocolFee});

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2Pool.sol#L175

Vulnerability details

Impact

Attacker (MEV bots) can take advantage of users on the chain of TimeswapV2Pool contract

Proof of Concept

The initialize, transferFees and transferLiquidity are critical functions that :
initialize : initializes the [strike][maturity] pool with the specified rate, can front ran and attacker who have access the mempool can override initialized the [strike][maturity] pool initialization rate
transferFees : transfers fees of the specified [strike][maturity] pool to the to address, attacker can front ran this and change the to to his address and theft the long0Fees, long1Fees, shortFees fees
transferLiquidity : transfers the liquidityAmount amount to address to of the specified [strike][maturity] pool, attacker can front ran this and change the to address to his address and theft the liquidityAmount amount

Tools Used

Manual review

Recommended Mitigation Steps :

make access control on the functions

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L202

Vulnerability details

Impact

function collectTransactionFees(TimeswapV2PoolCollectParam calldata param) external override noDelegateCall

as you can see there is not any access control on the above function and collectTransactionFees can get called by anyone, and any user can pass any param as input.

so malicious users can collect Transaction Fees,
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L207

and then call collect
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L215

to transfer the Position to any recipient.

Proof of Concept

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L202
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L207
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L215
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L232

Tools Used

manually

Recommended Mitigation Steps

this function should be callable only by the owner, and the owner account should point to a multisig managed by the Sandclock team or by a community DAO.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-option/src/TimeswapV2OptionDeployer.sol#L32-L35
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25-L29

Vulnerability details

Impact

when try to deploy pool or option contract if created address has been deployed before, there is no chance to create this pair.

Proof of Concept

Due to creation of pool and option contracts are deterministic and there is no chance to create with different address in optionFactory and deploymentFactory contracts,it may behave unexpected in future because of the generated address has probably been created in ethereum chain before by someone.This situation cause EVM error revert. At that position there is no mechanism to deploy with different address so desired pair pool or option contract can't be created in any way.
This issue discussed in ethereum/EIPs#684.

Tools Used

Recommended Mitigation Steps

Can be given permission to owner for change create mechanism if creating of specific pair is reverted. Another way is use a nonce value.Start with 0 and if contract creation is reverted increase nonce value 1.
There is a exapmle code for using nonce escape mechanism.
https://imgur.com/sTa3Vwd

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L252-L257

Vulnerability details

Impact

function mint(
    TimeswapV2PoolMintParam calldata param,
    bool isQuote,
    uint96 durationForward
) private noDelegateCall returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {

The contract uses the NoDelegateCall library to prevent delegatecall attacks, however, it does not check if the msg.sender or the msg.data have been modified before the internal call. This means that an attacker could potentially modify the msg.sender or msg.data before the internal call, allowing them to perform a delegatecall attack.

Proof of Concept

Attacker will create a malicious contract that modifies the msg.sender or msg.data before calling the vulnerable contract's internal function. This would then allow the attacker to execute malicious code within the context of the vulnerable contract or call external contracts in an unintended way.

Code Snippet.

pragma solidity ^0.8.0;

contract Attacker {
    address victim;
    bytes memory maliciousData;

    constructor(address _victim) public {
        victim = _victim;
    }

    function executeAttack() public {
        // Modify msg.sender
        address originalSender = msg.sender;
        msg.sender = address(this);

        // Modify msg.data
        bytes memory originalData = msg.data;
        maliciousData = new bytes(originalData.length);
        assembly {
            mstore(add(maliciousData, 0x20), originalData)
        }
        msg.data = maliciousData;

        // Perform the internal call
        victim.delegatecall(msg.data);

        // Revert the changes to msg.sender and msg.data
        msg.sender = originalSender;
        msg.data = originalData;
    }
}

The Attacker contract is able to modify the msg.sender and msg.data before performing an internal call to the victim contract. If the victim contract is using the NoDelegateCall library without checking the msg.sender and msg.data, an attacker can potentially exploit this vulnerability to execute malicious code in the context of the victim contract.

Tools Used

Manual audit, Remix, Vs Code.

Recommended Mitigation Steps

Use a library or a contract that has been specifically designed to check if the msg.sender and the msg.data have been modified before an internal call. For example, the OpenZeppelin's SafeMath library has a built-in check for this and can be used to replace the NoDelegateCall library in the contract.

Code.

pragma solidity ^0.8.0;
import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/math/SafeMath.sol";

contract MyContract {
    using SafeMath for uint256;

    function internalCall() internal {
        // msg.sender and msg.data have been checked for modification before this internal call
        // Your internal call here
    }

    function externalCall() public {
        // msg.sender and msg.data have been checked for modification before this internal call
        internalCall();
    }
}

Another approach would be to check the msg.data and msg.sender before any internal call.

Code.

pragma solidity ^0.8.0;

contract MyContract {

    function internalCall() internal {
        // Your internal call here
    }

    function externalCall() public {
        require(msg.sender == address(this), "Sender is not the contract owner");
        require(msg.data == "0x", "msg.data has been modified");
        internalCall();
    }
}

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2LiquidityToken.sol#L150-L179

Vulnerability details

Impact

Anyone can burn tokens in TokenswapV2LiquidityToken.sol. I believe it is assumed that most critical functions in TimeswapV2LiquidityToken.sol can only be called by Liquidity Providers. But there is no check/protection to ensure that.

Proof of Concept

The External function burn() is accessible by everyone.

Tools Used

Manual Review.

Recommended Mitigation Steps

put a check to ensure that minting can only be carried out by the liquidity Providers.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2LiquidityToken.sol#L75-L96

Vulnerability details

Impact

in TimeswapV2LiquidityToken.sol, the external function transferFeesFrom() is not protected to make sure that it's only accessible to the liquidity providers.
Therefore anyone under the guise of msg.sender can call it and earn/transfer the transaction fees meant only for the liquidity providers to themselves.

Proof of Concept

Praise a hacker notices that transferFeesFrom isn't strictly restricted to the liquidity providers and can be called by the msg.sender.
Praise then calls the external function transferFeesFrom externally from another function with this.myFunc(), which allows him to change the msg.sender from the account that signed the transaction to the contract itself.
He then transfers all the fees to himself.

Tools Used

Manual Review

Recommended Mitigation Steps

Put a Restriction on this function to make it strictly accessible only to the liquidity providers, not just msg.senders.
Here is what i mean, you can create another contract specifically representing Liquidity Providers and strictly restrict the accessibility of external functions like transferFeesFrom to it.
Or put a check to make sure that only EOA's can call it and not contracts.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L184

Vulnerability details

Impact

if any single account has its private keys compromised or decides to become malicious on its own, a single account can collect all fees on protocol and send it to any recipient address.

Proof of Concept

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L189

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L231

this is truly centralized risk and similar to this can be found here:
https://code4rena.com/reports/2022-01-sandclock/#m-14-a-single-malicious-trusted-account-can-takeover-parent-contract
https://code4rena.com/reports/2022-03-joyn/#h-05-centralisation-risk-owner-of-royaltyvault-can-take-all-funds
https://code4rena.com/reports/2022-11-non-fungible#m-02-hacked-owner-or-malicious-owner-can-immediately-steal-all-assets-on-the-platform
https://code4rena.com/reports/2022-01-insure/#h-10-a-maliciouscompromised-registry-or-factory-admin-can-drain-all-the-funds-from-the-vault-contracts

Tools Used

manually

Recommended Mitigation Steps

The owner account should point to a multisig managed by the Sandclock team or by a community DAO.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2Pool.sol#L155

Vulnerability details

Impact

This if statement's condition -

if (blockTimestamp(0) > maturity) Error.alreadyMatured(maturity, blockTimestamp(0)); 

will never be met, because blockTimeStamp is initialised to zero. block.TimeStamp being zero will never be greater than maturity.

Proof of Concept

block.TimeStamp being zero will never be greater than maturity.
So let's say maturity is "1" for instance being a uint. 0 > 1 will always be false. so the check for maturity, that is to know if it's already matured, is invalid.

Tools Used

Manual Review.

Recommended Mitigation Steps

block.TimeStamp shouldn't be initialized to zero.

See the markdown file with the details of this report here.

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-token/src/TimeswapV2LiquidityToken.sol#L70-L72

Vulnerability details

Impact

Anyone can use this critical external function transferTokenPositionFrom() found in TimeswapV2LiquidityToken.sol.
This function was meant to be used by only the liquidity providers. It transfers the position of tokens used by liquidity providers to add liquidity to pools.
so anyone can transfer the position of these tokens from one address to another.

Proof of Concept

The critical external function is accessible to everyone.

Tools Used

Manual Review.

Recommended Mitigation Steps

put a restriction to make the function solely accessible to the liquidity providers alone.