Summary
According to docs there are 2 ways of exit short:
Primary: user submits market bid to buy ercDebt for collateral and therefore repay. User receives collateral on partial exit
Secondary: user provides ercDebt for collateral. Issue is that user doesn't receive collateral on partial exit
Vulnerability Details
User's ethEsrowed increases only when buyBackAmount == ercDebt, otherwise he doesn't receive repayed collateral
function exitShortWallet(address asset, uint8 id, uint88 buyBackAmount)
external
isNotFrozen(asset)
nonReentrant
onlyValidShortRecord(asset, msg.sender, id)
{
...
if (buyBackAmount == ercDebt) {
uint256 vault = s.asset[asset].vault;
uint88 collateral = short.collateral;
s.vaultUser[vault][msg.sender].ethEscrowed += collateral;
LibShortRecord.disburseCollateral(
asset, msg.sender, collateral, short.zethYieldRate, short.updatedAt
);
LibShortRecord.deleteShortRecord(asset, msg.sender, id);
} else {
short.ercDebt -= buyBackAmount;
short.maybeResetFlag(asset);
}
...
}
Impact
User expects to receive collateral on partial exit like it happens in Secondary Exit Short. This non-obvious behaviour leads to poor UX and isn't documented.
Tools Used
Manual Review
Recommendations
if (buyBackAmount == ercDebt) {
uint256 vault = s.asset[asset].vault;
uint88 collateral = short.collateral;
s.vaultUser[vault][msg.sender].ethEscrowed += collateral;
LibShortRecord.disburseCollateral(
asset, msg.sender, collateral, short.zethYieldRate, short.updatedAt
);
LibShortRecord.deleteShortRecord(asset, msg.sender, id);
} else {
short.ercDebt -= buyBackAmount;
+ s.vaultUser[vault][msg.sender].ethEscrowed += buyBackAmount.mul(LibOracle.getPrice(asset));
short.maybeResetFlag(asset);
}