How to get right msg.sender calling from contract instance a.foo() and have all secure from 3rd parties

Question

I got stucked in thoughts guys and I need bit of clarification to move it forward.

(Description is about real problem, code is for best (I hope so..) understanding me)

I have main contract A that uses erc20 token and its transfer functions, there is also inherited Ownable and whitelist with accessing addresses of contracts e.g. B, I passing A address to the B constructor

I created instance of A in B (because in B I calculate collateral and I want to split to two different contracts)

Protecting A using Ownable it's enough from accessing A from 3rd party while interacting with A from A, Ownable in B isnt work because can't use right msg.sender, without modifiers someone can create instance of B in his contract and have access to my A, isn't it?

Thanks in advance

EDIT: I change a code for more to understand

// SPDX-License-Identifier: MIT
pragma solidity 0.8;

contract Ownable {
    mapping (address => bool) private whiteListed;
    modifier onlyWhiteListed {
        require(whiteListed[msg.sender] == true, "You're not whitelisted");
        _;
    }

    constructor () {
        whiteListed[msg.sender] = true;
    }

    function setWhiteListed(address addr, bool trueOrFalse) public onlyWhiteListed {
        whiteListed[addr] = trueOrFalse;
    }

    function getWhiteListed(address addr) public view returns (bool) {
        return whiteListed[addr];
    }
}

contract A is Ownable {
    B public b;
    event LogMsgSender(address who);

    constructor() {
        b = new B(address(this));
        //setWhiteListed(address(this),true);
        setWhiteListed(address(b),true);
    }

    function callMe() public onlyWhiteListed {  // here I can only get real caller msg.sender 
        emit LogMsgSender(msg.sender);          // when interact with that contract, not outside of contract
    }

    function callSender(address sender) public onlyWhiteListed {    // here I can get real caller msg.sender from other contract
        emit LogMsgSender(sender);                                  // but is it worth to passing addresses ?
    }                                                               // and HERE is my question: is this aproach secure??
}

contract B is Ownable {
    A public a;

    constructor(address addr) {   //in remix I can't deploy in one time while 
        a = A(addr);              //deploying A, I have to deploy copying addresses of A and do it separately 
    }                             //and after deploying also setWhiteListed() to whiteList in A

    function callMe() public onlyWhiteListed {  // this call isn't good for interact with sender, it's exlusive for calling as contract B
        a.callMe();                             // copies of this contract can 
    }

    function callSender() public onlyWhiteListed {  // modifiers protect from 3rd party contracts but 
        a.callSender(msg.sender);                   // that way it only can be used by deployer and whitelisted, (without modifiers they aren't secure)
    }                                               // bad idea ofc is adding to whitelist all over and over and
                                                    // it's impossible to recognize which to add which not
}

contract C {
    B public b;

    constructor(address addr) {
        b = B(addr);
    }

    function callMe() public {  //when modifiers is added this contract can't call these functions
        b.callMe();             // but with modifiers functions can't be used to get right sender address
    }                           

    function callSender() public {
        b.callSender();
    }
}

So now I'm deciding to inherite e.g. B is A and I simply get right sender, what do you think?

Answer 1

You've got a lot of syntax/logic errors in the code you posted. Not only are you missing onlyOwner modifiers, but your calls in B are via the type A: A.receiveToken() is not the same as a.receiveToken() ... so you've probably made several typos in posting your question (Should say a=A(aAddr); in the constructor etc).

The msg.sender will be "b" when b calls a.sendToken() but tx.origin will be unchanged. Checking msg.sender checks which contract called you. In other words, it is checking (inside onlyOwner of A.sendToken) whether b is the one that called a.sendToken().

Start by fixing all the errors and then insert some debug logging for yourself, so you can see how msg.sender is changing during the calls. You can also log tx.origin to see how that remains the original sender.

The point is that the owner of the A that B created is that particular instance of B (b). It's therefore "natural" that inside B, it can call a.whatever() as a's owner.

Answer 2

You didn't specify, but if you are using the Ownable contract from openzeppelin you need to pass the modifier onlyOwner for the methods that you want to be protected. Ex:

function sendToken(uint value) public onlyOwner {
    //transferFrom
    emit Sent(value);
}