Solidity: Global variable msg.sender incorrect during inter-transaction function call

[dexterchan]

Description

A smart contract having a transaction function A calling transaction function B
Say, A() -> B()
Ideally, global variable msg.sender should be consistent in both A() and B() pointing to same caller address.
If End user "Mickey" call A(), the msg.sender of A() and B() should return address of Mickey.
In my finding, msg.sender return different result:
A() -> Mickey address
B() -> Smart Contract address of B

This issue was happening in both cases:
A(),B() in the same contract
A(),B() in different contract

Impact

this issue impact use case with Solidity 0.5

• Escrow contract with an Ethereum ERC20 token
  where transaction function from Escrow contract calling ERC20 transaction function:
  transferfrom, transfer function

Environment

• Compiler version:
  0.5.2+commit.1df8f40c.Emscripten.clang
  0.5.4+commit.9549d8ff.Emscripten.clang
  0.5.4+commit.9549d8ff.Linux.g++ (run from docker)
• Target EVM version (as per compiler settings):
  0.5.2+commit.1df8f40c.Emscripten.clang
  0.5.4+commit.9549d8ff.Emscripten.clang
  0.5.4+commit.9549d8ff.Linux.g++ (run from docker)
• Framework/IDE (e.g. Truffle or Remix):
  Remix, web3 1.0.0-beta.35
• EVM execution environment / backend / blockchain client:
  Remix, web3 1.0.0-beta.35
• Operating system:
  MACOS 10.14.3

Steps to Reproduce

Without loss of generality, I picked single contract case to illustrate the issue.

Sample contract A() , B() in same contract
A function is called freeze()
B function is called frozeCoin()

freeze() is calling frozeCoin() in below contract.
In Remix, it returns:
transact to TestMyToken.freeze errored: VM error: revert.
revert The transaction has been reverted to the initial state.
Reason provided by the contract: "owner required". Debug the transaction to get more information.

In web3, it returns

VM Exception while processing transaction: revert owner required
`
pragma solidity ^0.5.0;
contract TestMyToken {

bool public frozen;
address public owner;
constructor() public {
    owner=msg.sender;
    frozen=false;
}
function frozeCoin() public {
    require(owner==msg.sender,"owner required");
    require(!frozen, "Coin frozen already");
    frozen=true;
}
function freeze() public {
    require(owner==msg.sender,"owner required");
    require(!frozen, "Coin freeze");
    //the caller became the contract itself instead of end user account
    this.frozeCoin();
}
modifier checkFrozen(){
        require(!frozen,"Coin is frozen");
        _;
}

}
`

[axic]

this.frozeCoin();

If you call your other function via this. that becomes and external call and the msg.sender is the contract.

Just call frozeCoin() without this.

[dexterchan]

Thanks, axic.... it solved the case of two function in same contract...
what if, the two functions are from different contract?

• MyToken
• TokenInteraction

In Remix, using account A, I deployed Mytoken....
Copying the contract address, with same account A, I deploy TokenInteraction...
If I call "transfer" in Mytoken contact to accountB, it is ok...
But, with TokenInteraction, account A "transferToken", we got error:

transact to TokenInteraction.transferToken errored: VM error: revert.
revert The transaction has been reverted to the initial state.
Reason provided by the contract: "owner required".
Clearly, msg.sender in "transferToken" gives contract address..... it is not expected......

This scenario has issue in escrow ERC20 use case.... using another contract to transfer ERC20 token within ERC20 contract.
With this issue, in withdraw , ERC20 contract will call "msg.sender" and take the contract contract as sender address incorrectly(suppose to be original owner address)
Since, the contract address has zero balance, the transaction threw exception and failed to put remaining token back to original owner. In the step of deposit, the transaction will be incorrect but not throwing exception.
Reference:
https://programtheblockchain.com/posts/2018/05/30/escrowing-erc20-tokens/

``
contract MyToken is ERC20 {
bool public frozen;
address public owner;
constructor(uint initial) public {
owner=msg.sender;
// Initially assign all tokens to the contract's creator.
_mint(msg.sender, initial);
frozen=false;
}
function transfer(address _to, uint256 _value) public restrictOwner returns (bool success) {
_transfer(msg.sender, _to, _value);
return true;
}

modifier restrictOwner(){
    require(owner==msg.sender,"owner required");
        _;
}

}

pragma solidity ^0.5.0;

import "./MyToken.sol";

contract TokenInteraction {

address public tokenAddress;
address public owner;
constructor(address _tokenAdd) public {
    owner=msg.sender;
    tokenAddress = _tokenAdd;
}

function transferToken(address to) public restrictOwner {
    MyToken myToken = MyToken(tokenAddress);
    myToken.transfer(to, 1);
}

modifier restrictOwner(){
    require(owner==msg.sender,"owner required");
        _;
}

}
``

[axic]

I think the best way to discuss questions like this is https://ethereum.stackexchange.com or https://gitter.com/ethereum/solidity

[dexterchan]

thank you, let me exchange idea with other experts in this area. May come back to u soon.