Approval called within a contract set the allowance but the transferFrom fail for insuficient allowance

Support Contracts
1

Greetings!

I am testing a smart contract suite with hardhat.

I want to test reentrance, so here is my scenario.

I have a contract A. It's an ERC20.sol from OZ.
I have a contract B that is made to call my Token contract A
The contract B have the interface to call A.

The owner of Contract B makes the sellShares call to B, the victim is A:

As per the logs The address is right and the allowance as well.

function sellShares() public payable {
        approveVictim();
        uint256 allowance = victim.allowance(address(this), address(victim));
        console.log(allowance);
        console.log(address(this));
        victim.sellShares(1000000000000000000);
    }

    function approveVictim() public{
        victim.approve(address(victim), 400000000000000000000000);
    }

This call gets sent to the victim Contract A

function sellShares(uint256 amount) public nonReentrant {
        console.log(allowance(msg.sender, address(this)));
        console.log(msg.sender);
        transferFrom(_msgSender(), address(this), amount);
}

Here again the values for the address and allowance are as expected.

I initiate this call from my test file

   let allowance = await www3Shares.allowance(B.target, A.target)
                console.log("=>",allowance)
                console.log("=>",B.target)

Here as well, the values are as expected.

It keeps reverting with

reverted with reason string 'ERC20: insufficient allowance

:1234: Code to reproduce

:computer: Environment

"hardhat": "^2.18.2",
node v16.17.1
"@openzeppelin/contracts": "^4.8.3",

2 
 function approveVictim() public{
        victim.approve(address(victim), 400000000000000000000000);
    }

Are you trying to approve the same contract ? victim approve victim ?

3

Yes I am. I'm approving the token contract.

But I did more testing and I'm also unable to approve that same contract to allow it to transfer from an account.

In all cases the allowance get set but...

Just pushed this case.

4

I have updated to Openzeppelin contract 5.0.0

It is still failing but with this error now;

Error: VM Exception while processing transaction: reverted with an unrecognized custom error (return data: 0xfb8f41b2000000000000000000000000f39fd6e51aad88f6f4ce6ab8827279cfffb9226600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000015af1d78b58c40000)

5

This looks like the allowance is still 0!

6

You should execute approve from your (offchain) script, not from your (onchain) contract.

7

The contract is the owner of the of the tokens. So the contract must approve another contract, in this case the token contract itself.

I have also tested from an account instead. Same result.

I Think it might be related to Hardhat or other packages in the project

8

x.approve(address(x), amount) is never the right thing to do.

An account, whether it's an externally-owned account (aka wallet) or a smart-contract account, never needs to approve itself to transfer an amount from itself, as it can simply transfer that amount without any approval.

When you take money out of your own wallet, do you ask yourself for a permission (approval) beforehand?

9

A contract needs approval to transfer funds from another contract or an account.

In this case the contract that need approval happens to be the token contract.

It will need an approval from an EOA or another contract.

tokenContract.approve() // <- we make the approval call to the contract

In my case. I approve the token contract to pull tokens out of my wallet/contract

tokenContract.approve(address(tokenContract), amount);

Same thing with an EOA

await token.connect(owner).approve(token.target, ethers.parseEther("5"))

Here, as the owner of the token in my wallet, I approve the token contract to pull tokens out of my wallet.

Here is a minimal reproduction.

https://github.com/Madeindreams/transfer-case/tree/main

10

@barakman

I don't see why I could not approve the token contract to take tokens from my wallet/contract.
And if it was the case, Why is it not transferring the tokens regardless of the allowance?

I don't see any logic in the ERC20 contract that would prevent the token contract from transferring to itself or setting itself as the spender.

11

I found it @barakman

function transferFrom(address from, address to, uint256 value) public virtual returns (bool) {
    address spender = _msgSender();
    _spendAllowance(from, spender, value);
    _transfer(from, to, value);
    return true;
}

This is what makes it fail. Transfer from can only be called externally.