Uploaded files for judging

001.md

@@ -0,0 +1,48 @@
+Urban Fiery Guppy
+
+High
+
+# createOrder will be abused to drain all approvals
+
+### Summary
+createOrder uses generateOrderId, which returns the same value within one block for the same recipient:
+```solidity
+    ///@notice generate a random and unique order id
+    function generateOrderId(address sender) external view override returns (uint96) {
+        uint256 hashedValue = uint256(
+            keccak256(abi.encodePacked(sender, block.timestamp))
+        );
+        return uint96(hashedValue);
+    }
+```
+As there is no access control for order creation, and funds are transferred from the recipient (not msg.sender), it is possible to drain all approvals by creating orders with `minAmountOut = 0`.
+
+### Root Cause
+
+Missing `msg.sender == order.recipient` check in `createOrder`; usage of only `recipient` and `block.timestamp` for orderId derivation.
+
+### Internal pre-conditions
+
+_No response_
+
+### External pre-conditions
+
+Any user who approves any whitelisted token to the OracleLess/Bracket/StopLimit contract. 
+
+### Attack Path
+
+1. User approves 10_000 USDC to OracleLess contract, creates an order for 10_000 USDC
+2. Attacker's contract iterates through pendingOrderIds, starting from the last element, records all orders that were created during the current block.
+3. In the same transaction, for each such order, Attacker's contract calls `createOrder`, with the same data but `minAmountOut = 0`. `generateOrderId` generates the same id, because it depends only on the `block.timestamp` and `recipient`, so the attacker's `createOrder` overwrites the order created by the victim. 
+
+4. In the same transaction, attacker's contract calls `fillOrder`, and swaps all victim's funds for 0.
+
+Alternatively, attacker can monitor approvals of whitelisted tokens to the contract, and `createOrder` with `minAmountOut = 0`, and `amount = approvedAmount`. 
+
+### Impact
+
+All users who approved a whitelisted token to contracts OracleLess/Bracket/StopLimit, will lose the whole approved amount.
+
+### Mitigation
+
+Use nonce for deriving orderId, and validate msg.sender == recipient.

002.md

@@ -0,0 +1,66 @@
+Crazy Smoke Barbel
+
+High
+
+# generateOrderId can lead to order overwrite allowing an attacker to drain the tokensIn
+
+### Summary
+
+`generateOrderId` uses user supplied `recipient` in `Oracleless.sol` which can lead to an overwrite in `orders` but duplicate value in `pendingOrderIds` which can allow to withdraw more tokens than there were sent in
+
+### Root Cause
+
+In `Oracleless.sol` the function [`createOrder()`](https://github.com/sherlock-audit/2024-11-oku/blob/e844037b3fcd8288efe10a2f1cf43e62bad7b4e1/oku-custom-order-types/contracts/automatedTrigger/OracleLess.sol#L38) allows the attacker to control recipient value which gets passed on to `MASTER.generateOrderId(recipient);` which allows an attacker to make 2 requests that will have the same `orderId` overwriting the orders in [ ` orders[orderId] = ...`](https://github.com/sherlock-audit/2024-11-oku/blob/e844037b3fcd8288efe10a2f1cf43e62bad7b4e1/oku-custom-order-types/contracts/automatedTrigger/OracleLess.sol#L52) but it ends up twice in[ `pendingOrderIds`](https://github.com/sherlock-audit/2024-11-oku/blob/e844037b3fcd8288efe10a2f1cf43e62bad7b4e1/oku-custom-order-types/contracts/automatedTrigger/OracleLess.sol#L64) because it's an array instead of a mapping. In this case an attacker can send `1 tokenIn` on the first request and `2 tokenIn` on the second request and when calling `cancelOrder` twice with the `orderId` the contract will send 2 * `2 tokenIn`, more than it was sent initially in the contract.
+
+### Internal pre-conditions
+
+The attacker has to call `createOrder` twice and then `cancelOrder` twice in the same block (can be done in same transaction)
+
+### External pre-conditions
+
+_No response_
+
+### Attack Path
+
+1. Call `createOrder` with 1 token
+2. Call `createOrder` with 2 tokens
+3. Call `cancelOrder` -> receive 2 tokens
+4. Call `cancelOrder` -> receive 2 tokens
+
+### Impact
+
+The attacker can withdraw more tokens than he initially sent during `createOrder`
+
+### PoC
+
+```solidity
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import {OracleLess} from "OracleLess.sol";
+
+contract Exploit {
+    OracleLess target;
+	address tokenIn;
+	address tokenOut;
+	
+    constructor(address adr, address tIn, address tOut) {
+        target = OracleLess(adr);
+		tokenIn = tIn;
+		tokenOut = tOut;
+    }
+
+    function exploit() public {
+        target.createOrder(tokenIn, tokenOut, 1, 0, address(this), 0);
+        target.createOrder(tokenIn, tokenOut, 2, 0, address(this), 0);
+        uint96 orderId = target.generateOrderId(address(this));
+        target.cancelOrder(orderId);
+        target.cancelOrder(orderId);
+    }
+}
+```
+
+### Mitigation
+
+Option 1: Don't allow user supplied data in `generateOrderId`
+Option 2: Check if `orderId` already exists in `orders` in `createOrder` in `OracleLess.sol`

003.md

@@ -0,0 +1,80 @@
+Orbiting Goldenrod Jaguar
+
+High
+
+# Incorrect Oracle Validation will Allow Invalid Order Creation
+
+### Summary
+
+Incorrect oracle validation logic will cause an invalid order creation vulnerability for users as malicious actors will create orders with non-existent tokenOut oracles, leading to potential price manipulation and failed executions.
+
+
+### Root Cause
+
+https://github.com/sherlock-audit/2024-11-oku-Lu-zihang/blame/e8a9b54b988b021a707e13b8f8048b1b9cc1d602/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L458-L459
+In Bracket.sol:459 there is a duplicate check of tokenIn oracle instead of checking tokenOut oracle in the require statement.
+
+
+### Internal pre-conditions
+
+_No response_
+
+### External pre-conditions
+
+_No response_
+
+### Attack Path
+
+_No response_
+
+### Impact
+
+The users suffer from incorrect order execution and potential fund loss, as the missing tokenOut oracle validation affects these critical functions:
+
+1. `checkInRange()` - Uses invalid oracle:
+`exchangeRate = MASTER.getExchangeRate(order.tokenIn, order.tokenOut);`
+
+2. Order creation - Sets wrong direction:
+`direction: MASTER.getExchangeRate(tokenIn, tokenOut) > takeProfit`
+
+3. `execute()` - Calculates wrong minimum received amount:execute() - Calculates wrong minimum received amount:
+`MASTER.getMinAmountReceived(amountIn, tokenIn, tokenOut, bips)`
+
+4. Exchange Rate Calculation Failure:
+```solidity
+function _getExchangeRate(IERC20 tokenIn, IERC20 tokenOut) internal view returns (uint256) {
+       uint256 priceIn = oracles[tokenIn].currentValue();
+>@     uint256 priceOut = oracles[tokenOut].currentValue(); // Will revert if oracle doesn't exist
+       return (priceIn * 1e8) / priceOut;
+}
+```
+
+### PoC
+
+_No response_
+
+### Mitigation
+
+```solidity
+    function _createOrder(
+        uint256 takeProfit,
+        uint256 stopPrice,
+        uint256 amountIn,
+        uint96 existingOrderId,
+        IERC20 tokenIn,
+        IERC20 tokenOut,
+        address recipient,
+        uint16 feeBips,
+        uint16 takeProfitSlippage,
+        uint16 stopSlippage
+    ) internal {
+        //verify both oracles exist, as we need both to calc the exchange rate
+        require(
+            address(MASTER.oracles(tokenIn)) != address(0x0) &&
+           -    address(MASTER.oracles(tokenIn)) != address(0x0),
+           +    address(MASTER.oracles(tokenOut)) != address(0x0),
+            "Oracle !exist"
+        );
+        ........
+
+```

004.md

@@ -0,0 +1,52 @@
+Fresh Topaz Flamingo
+
+Medium
+
+# Strict inequality in swap validation will cause unnecessary reverts for users
+
+## Summary
+
+The `execute` function implements a swap mechanism where the received amount of `tokenOut` is validated against a minimum threshold calculated by `MASTER.getMinAmountReceived`. The comparison currently uses a strict greater-than (`>`) operator instead of a greater-than-or-equal-to (`>=`). This causes the function to revert even when the minimum required amount is received, especially when `bips` is set to `0`.
+
+https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L549-L560
+
+## Root Cause
+
+The conditional statement in `execute`:
+
+```solidity
+require(
+    finalTokenOut - initialTokenOut >
+        MASTER.getMinAmountReceived(amountIn, tokenIn, tokenOut, bips),
+    "Too Little Received"
+);
+```
+
+uses a strict greater-than (`>`) comparison to enforce the minimum amount constraint. 
+**Looking at the meaning of the `slippageBips` variable in the `getMinAmountReceived` function, it means that when the user wants to convert exactly at `exchangeRate`, user sets `slippageBips` to 0.**
+When `bips` is `0`, the `getMinAmountReceived` function returns the exact calculated fair value. If the received amount is exactly equal to this value, the condition fails, leading to an unnecessary revert.
+
+This issue stems from an incorrect logical assumption that the received amount must exceed the calculated minimum when, in fact, equality should suffice.
+
+
+## Impact
+
+- **Unnecessary Reverts:** If `bips` is `0`, valid transactions where the received amount matches the expected minimum will revert, resulting in failed swaps.
+- **Disruption of Operations:** This issue may block swaps in low-slippage or exact-match scenarios, causing inefficiencies in trading systems relying on this function.
+- **User Frustration:** Users might encounter unexpected failures without a clear understanding of the underlying reason.
+
+
+## Mitigation
+
+Replace the `>` operator with `>=` in the `require` statement within the `execute` function:
+
+```solidity
+require(
+    finalTokenOut - initialTokenOut >=
+        MASTER.getMinAmountReceived(amountIn, tokenIn, tokenOut, bips),
+    "Too Little Received"
+);
+```
+
+This change ensures that swaps proceed when the received amount meets or exceeds the calculated minimum, accommodating scenarios with zero slippage or exact matching.
+