sherlock-audit/2023-04-blueberry-judging

BugHunter101 - takeCollateral() doesn't check whether the caller actually has a corresponding debt position in the bank

Closed this issue · 0 comments

BugHunter101

high

takeCollateral() doesn't check whether the caller actually has a corresponding debt position in the bank

Summary

this function is that it does not check whether the caller actually has a corresponding debt position in the bank. This means that an attacker could potentially call this function with a large amount value to drain the bank's collateral without actually having any debt to repay.

Vulnerability Detail

this function is that it does not check whether the caller actually has a corresponding debt position in the bank. This means that an attacker could potentially call this function with a large amount value to drain the bank's collateral without actually having any debt to repay.

Impact

an attacker could potentially call this function with a large amount value to drain the bank's collateral without actually having any debt to repay.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L793

function takeCollateral(
uint256 amount
) external override inExec returns (uint256) {
Position storage pos = positions[POSITION_ID];
if (amount == type(uint256).max) {
amount = pos.collateralSize;
}
pos.collateralSize -= amount;
IERC1155Upgradeable(pos.collToken).safeTransferFrom(
address(this),
msg.sender,
pos.collId,
amount,
""
);
emit TakeCollateral(
POSITION_ID,
msg.sender,
pos.collToken,
pos.collId,
amount
);

    return amount;
}

Tool used

Manual Review

Recommendation

In this implementation, the function first checks whether the caller has a corresponding debt position by verifying that the debtShare value is non-zero. If it is zero, then the function reverts with an error message indicating that the caller does not have a debt position. If the debtShare value is non-zero, then the function proceeds with transferring the requested amount of collateral back to the caller.

example:

function takeCollateral(uint256 amount) external override inExec returns (uint256) {
Position storage pos = positions[POSITION_ID];
if (amount == type(uint256).max) {
amount = pos.collateralSize;
}
if (pos.debtShare == 0) {
revert Errors.NO_DEBT_POSITION();
}
pos.collateralSize -= amount;
IERC1155Upgradeable(pos.collToken).safeTransferFrom(
address(this),
msg.sender,
pos.collId,
amount,
""
);
emit TakeCollateral(
POSITION_ID,
msg.sender,
pos.collToken,
pos.collId,
amount
);

return amount;

}