code4rena-2024-03-dittoeth-m01
[M-01] The shortOrder verification bug on the RedemptionFacet.proposeRedemption() allows an attacker to leave a small shortOrder on the order book, leading to the protocol’s bad debt
Summary
호출자의 파라미터를 먼저 검증하지 않고 사용했기 때문에 청산시 소액이 남은 숏 주문을 닫지 않도록 할 수 있다. 이 주문이 추후 체결되면 작은 포지션의 숏을 만들고, 작은 포지션의 숏은 청산될 수도 없어 불량채권이 될 수 있다.
Keyword
lack of input validation, defi
Vulnerability
새로운 구매 요청이 들어와 기존의 판매 주문과 매칭하는 BidOrdersFacet.bidMatchAlgo 에서 숏 주문이 부분매칭 되었을 때, 숏 주문에 남은 판매량이 LibAsset.minAskEth(asset).mul(C.DUST_FACTOR)) 보다 많다면 숏 주문을 닫지 않는다. DUST_FACTOR 는 0.5 ether로 이는 남은 판매량의 가치가 minAskEth(최소 주문 금액) 의 50% 이상이면 주문을 닫지 않고 남겨둔다는 의미이다. 이를 통해 남은 판매량이 minAskEth 보다 작은 주문이 남을 수 있다.
function bidMatchAlgo(
address asset,
STypes.Order memory incomingBid,
MTypes.OrderHint[] memory orderHintArray,
MTypes.BidMatchAlgo memory b
) private returns (uint88 ethFilled, uint88 ercAmountLeft) {
uint256 minBidEth = LibAsset.minBidEth(asset);
MTypes.Match memory matchTotal;
while (true) {
// @dev Handles scenario when no sells left after partial fill
...
STypes.Order memory lowestSell = _getLowestSell(asset, b);
if (incomingBid.price >= lowestSell.price) {
...
if (incomingBid.ercAmount > lowestSell.ercAmount) {
...
} else {
if (incomingBid.ercAmount == lowestSell.ercAmount) {
...
} else {
lowestSell.ercAmount -= incomingBid.ercAmount;
if (lowestSell.isShort()) {
b.dustShortId = lowestSell.id;
STypes.Order storage lowestShort = s.shorts[asset][lowestSell.id];
lowestShort.ercAmount = lowestSell.ercAmount;
} else {
b.dustAskId = lowestSell.id;
s.asks[asset][lowestSell.id].ercAmount = lowestSell.ercAmount;
}
// Check reduced dust threshold for existing limit orders
@> if (lowestSell.ercAmount.mul(lowestSell.price) >= LibAsset.minAskEth(asset).mul(C.DUST_FACTOR)) {
@> b.dustShortId = b.dustAskId = 0;
}
}
incomingBid.ercAmount = 0;
return matchIncomingBid(asset, incomingBid, matchTotal, b);
}
} else {
...
}
}
}담보율이 낮은 숏을 상환할 때, 상환 대상 숏이 부분 매칭되었고, 남은 판매량이 minShortErc(최소 숏 양) 보다 작다면 숏 주문을 닫는다. 즉 숏 주문에 남은 판매량이 minShortErc 이상이라면 숏 주문을 닫지 않는다. 추후 청산이 완료되어도 이 숏은 닫히지 않으므로 남아있는 숏 주문이 매칭되면 해당 숏에 다시 담보와 빚이 추가될 것이다.
그런데 여기서 문제가 있다. proposalInput 파라미터는 청산 대상에 대한 정보를 포함하며, 유저가 함수를 호출할 때 임의로 설정하는 파라미터이므로 검증이 필요하다. currentSR.status == SR.PartialFill && shortOrder.ercAmount < minShortErc 인 상황일 때만 shortOrder.shortRecordId != p.shortId || shortOrder.addr != p.shorter 인지 파라미터를 검증한다. 즉 공격자가 부분 매칭 상태가 아닌 숏 주문이나 남은 주문 양이 minShortErc 보다 높은 주문의 id를 p.shortOrderId 로 설정하면 파라미터 검증을 우회하며 원래 연동되어 있던 숏 주문을 닫지 않을 수 있다.
function proposeRedemption(
address asset,
@> MTypes.ProposalInput[] calldata proposalInput,
uint88 redemptionAmount,
uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
uint256 minShortErc = LibAsset.minShortErc(p.asset);
...
for (uint8 i = 0; i < proposalInput.length; i++) {
p.shorter = proposalInput[i].shorter;
@> p.shortId = proposalInput[i].shortId;
@> p.shortOrderId = proposalInput[i].shortOrderId;
...
// @dev Cancel attached shortOrder if below minShortErc, regardless of ercDebt in SR
// @dev All verified SR have ercDebt >= minShortErc so CR does not change in cancelShort()
@> STypes.Order storage shortOrder = s.shorts[asset][p.shortOrderId];
@> if (currentSR.status == SR.PartialFill && shortOrder.ercAmount < minShortErc) {
@> if (shortOrder.shortRecordId != p.shortId || shortOrder.addr != p.shorter) revert Errors.InvalidShortOrder();
@> LibOrders.cancelShort(asset, p.shortOrderId);
}
...
}
...
}닫히지 않고 열려있는 숏 주문이 추후 체결되면 극소량의 담보와 빚을 가진, 작은 포지션의 숏이 생성된다. 작은 포지션의 숏은 불량채권이 될 수 있다. 담보율이 떨어지더라도 shortRecord.ercDebt < minShortErc 인 포지션이 작은 숏은 청산할 수 없기 때문이다.
function validRedemptionSR(STypes.ShortRecord storage shortRecord, address proposer, address shorter, uint256 minShortErc)
internal
view
returns (bool)
{
// @dev Matches check in onlyValidShortRecord but with a more restrictive ercDebt condition
// @dev Proposer can't redeem on self
@> if (shortRecord.status == SR.Closed || shortRecord.ercDebt < minShortErc || proposer == shorter) {
return false;
} else {
return true;
}
}Impact
닫혀야 하는 숏 주문이 닫히지 않도록 할 수 있고, 이를 통해 프로토콜에 불량채권을 남길 수 있다.
Mitigation
파라미터를 먼저 검증한 뒤 사용하도록 한다.
function proposeRedemption(
address asset,
MTypes.ProposalInput[] calldata proposalInput,
uint88 redemptionAmount,
uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
...
for (uint8 i = 0; i < proposalInput.length; i++) {
p.shorter = proposalInput[i].shorter;
p.shortId = proposalInput[i].shortId;
p.shortOrderId = proposalInput[i].shortOrderId;
STypes.ShortRecord storage currentSR = s.shortRecords[p.asset][p.shorter][p.shortId];
...
STypes.Order storage shortOrder = s.shorts[asset][p.shortOrderId];
+ if (shortOrder.shortRecordId != p.shortId || shortOrder.addr != p.shorter) revert Errors.InvalidShortOrder();
if (currentSR.status == SR.PartialFill && shortOrder.ercAmount < minShortErc) {
- if (shortOrder.shortRecordId != p.shortId || shortOrder.addr != p.shorter) revert Errors.InvalidShortOrder();
LibOrders.cancelShort(asset, p.shortOrderId);
}
...
}
...
}tags: bughunting, dittoeth, smart contract, solidity, lack-of-input-validation-vul, defi, severity medium