ctf_sec - Bypass the blacklist restriction because the blacklist check is not done when minting or burning
Opened this issue · 5 comments
ctf_sec
high
Bypass the blacklist restriction because the blacklist check is not done when minting or burning
Summary
Bypass the blacklist restriction because the blacklist check is not done when minting or burning
Vulnerability Detail
In the whitepaper:
the protocol emphasis that they implement a blacklist feature for enforcing OFAC, AML and other account security requirements
A blacklisted will not able to send or receive tokens
the protocol want to use the whitelist feature to be compliant to not let the blacklisted address send or receive dSahres
For this reason, before token transfer, the protocol check if address from or address to is blacklisted and the blacklisted address can still create buy order or sell order
function _beforeTokenTransfer(address from, address to, uint256) internal virtual override {
// Restrictions ignored for minting and burning
// If transferRestrictor is not set, no restrictions are applied
// @audit
// why don't you not apply mint and burn in blacklist?
if (from == address(0) || to == address(0) || address(transferRestrictor) == address(0)) {
return;
}
// Check transfer restrictions
transferRestrictor.requireNotRestricted(from, to);
}this is calling
function requireNotRestricted(address from, address to) external view virtual {
// Check if either account is restricted
if (blacklist[from] || blacklist[to]) {
revert AccountRestricted();
}
// Otherwise, do nothing
}but as we can see, when the dShare token is burned or minted, the blacklist does not apply to address(to)
this allows the blacklisted receiver to bypass the blacklist restriction and still send and receive dShares and cash out their dShares
because the minting dShares is not blacklisted
a blacklisted user create a buy order with payment token and set the order receiver to a non-blacklisted address
then later when the buy order is filled, the new dShares is transferred and minted to an not-blacklisted address
because the burning dShares is not blacklisted
before the user is blacklisted, a user can frontrun the blacklist transaction to create a sell order and transfer the dShares into the OrderProcessor
then later when the sell order is filled, the dShares in burnt from the SellOrderProcess escrow are burnt and the user can receive the payment token
Impact
Bypass the blacklist restriction because the blacklist check is not done when minting or burning
Code Snippet
Tool used
Manual Review
Recommendation
implement proper check when burning and minting of the dShares to not let user game the blacklist system, checking if the receiver of the dShares is blacklisted when minting, before filling sell order and burn the dShares, check if the requestor of the sell order is blacklisted
do not let blacklisted address create buy order and sell order
Fixes for this should be considered in combination with #55 as it creates more opportunities for locking up orders.
PR #131 fix goods good
PR #126 only fix only check if the recipient or requestor is blocklisted by the asset token transferRestrictor
if (
BridgedERC20(orderRequest.assetToken).isBlacklisted(orderRequest.recipient)
|| BridgedERC20(orderRequest.assetToken).isBlacklisted(msg.sender)
) revert Blacklist();the protocol may want to consider checking if the orderRequest.recipient or msg.sender is blocklisted by the payment token as well
Additional blacklist checks in
Fix looks good!