ERC721Pool taker callback misreports quote funds whenever there was collateral amount rounding
Summary
atomicSwapCallback() now reports tokensTaken higher than one corresponding to quoteTokenAmount whenever the rounding took place as the excessQuoteToken is omitted.
Vulnerability Detail
ERC721Pool's take() atomicSwapCallback needs to have full quote amount, (result.quoteTokenAmount + result.excessQuoteToken), when there is a quote token part added to have the whole integer amount of the collateral asset.
Now the callback quote value, quoteTokenAmount, is inconsistent with the collateral amount tokensTaken when there was rounding.
Impact
The impact depends on the logic on callee_ side, but asset amounts are usually used in the downstream asset management logic, so misreporting such amounts can lead to the asset losses on the taker's side.
As that's the precondition, setting the severity to be medium.
Code Snippet
ERC721Pool's take() reports result.quoteTokenAmount and tokensTaken as take result to the callee_, ignoring collateral rounding part:
if (data_.length != 0) {
IERC721Taker(callee_).atomicSwapCallback(
tokensTaken,
result.quoteTokenAmount / _getArgUint256(QUOTE_SCALE),
data_
);
}
if (result.settledAuction) _rebalanceTokens(borrowerAddress_, result.remainingCollateral);
// transfer from taker to pool the amount of quote tokens needed to cover collateral auctioned (including excess for rounded collateral)
_transferQuoteTokenFrom(callee_, result.quoteTokenAmount + result.excessQuoteToken);
Tool used
Manual Review
Recommendation
Consider adding the excessQuoteToken to the reported value:
if (data_.length != 0) {
IERC721Taker(callee_).atomicSwapCallback(
tokensTaken,
- result.quoteTokenAmount / _getArgUint256(QUOTE_SCALE),
+ (result.quoteTokenAmount + result.excessQuoteToken) / _getArgUint256(QUOTE_SCALE),
data_
);
}
if (result.settledAuction) _rebalanceTokens(borrowerAddress_, result.remainingCollateral);
// transfer from taker to pool the amount of quote tokens needed to cover collateral auctioned (including excess for rounded collateral)
_transferQuoteTokenFrom(callee_, result.quoteTokenAmount + result.excessQuoteToken);
hyh
medium
ERC721Pool taker callback misreports quote funds whenever there was collateral amount rounding
Summary
atomicSwapCallback() now reports
tokensTaken
higher than one corresponding toquoteTokenAmount
whenever the rounding took place as theexcessQuoteToken
is omitted.Vulnerability Detail
ERC721Pool's take()
atomicSwapCallback
needs to have full quote amount,(result.quoteTokenAmount + result.excessQuoteToken)
, when there is a quote token part added to have the whole integer amount of the collateral asset.Now the callback quote value,
quoteTokenAmount
, is inconsistent with the collateral amounttokensTaken
when there was rounding.Impact
The impact depends on the logic on
callee_
side, but asset amounts are usually used in the downstream asset management logic, so misreporting such amounts can lead to the asset losses on the taker's side.As that's the precondition, setting the severity to be medium.
Code Snippet
ERC721Pool's take() reports
result.quoteTokenAmount
andtokensTaken
as take result to thecallee_
, ignoring collateral rounding part:https://github.com/sherlock-audit/2023-01-ajna/blob/main/contracts/src/ERC721Pool.sol#L452-L463
Tool used
Manual Review
Recommendation
Consider adding the
excessQuoteToken
to the reported value:https://github.com/sherlock-audit/2023-01-ajna/blob/main/contracts/src/ERC721Pool.sol#L452-L463