0

I am trying to figure out how to set up a simple communication between contracts.

For example, we have a contract Passport, in which the user writes his data. After that, through the Election contract, the user votes. The contract Election should check if the user has a passport and only then allow him to vote.

Here is how I am trying to do it:

Passport contract

contract Passport
{
    struct      Person
    {
        string  name;
        string  surname;
        uint8   age;
        uint256 id;
        bool    registered;
    }

    address payable public              owner;
    mapping(address => Person) public   people;
    uint256                             idCount;

    modifier ownerOnly()
    {
        require(msg.sender == owner);
        _;
    }

    constructor() public payable
    {
        owner = msg.sender;
    }

    function registerID(string memory _name, string memory _surname, uint8 _age) public
    {
        require(!people[msg.sender].registered);

        people[msg.sender].name = _name;
        people[msg.sender].surname = _surname;
        people[msg.sender].age = _age;
        people[msg.sender].id = idCount;

        people[msg.sender].registered = true;

        idCount += 1;
    }

    // I use this function to check is the user have passport
    function isRegistered(address _address) public view returns (bool)
    {
        return people[_address].registered;
    }

    function end() ownerOnly public
    {
        selfdestruct(owner);
    }
}

Election contract

contract Election
{
    struct      Candidate
    {
        string  name;
        uint    voteCount;
    }

    struct      Voter
    {
        bool    authorized;
        bool    voted;
        uint    voteTarget;
    }
    address payable  public             owner;
    string public                       electionName;
    mapping(address => Voter) public    voters;
    Candidate[] public                  candidates;
    uint public                         totalVotes;
    Passport                            pass;

    modifier ownerOnly()
    {
        require(msg.sender == owner);
        _;
    }

    constructor(string memory _electionName) public
    {
        owner = msg.sender;
        electionName = _electionName;
    }

    function addCandidate(string memory _candidateName) ownerOnly public
    {
        candidates.push(Candidate(_candidateName, 0));
    }

    function getNumCandidate() public view returns(uint)
    {
        return candidates.length;
    }

    function authorize(address _person) public
    {
        voters[_person].authorized = true;
    }

    function vote(uint _voteIndex) public
    {
        // Here I register pasport with current user address
        pass = Pasport(msg.sender);

        // Here I check if the user already have pasport
        require(pass.isRegistered(msg.sender));

        require(!voters[msg.sender].voted);
        require(voters[msg.sender].authorized);

        voters[msg.sender].voteTarget = _voteIndex;
        voters[msg.sender].voted = true;

        candidates[_voteIndex].voteCount += 1;
        totalVotes += 1;
    }

    function end() ownerOnly public
    {
        selfdestruct(owner);
    }
}

What is the correct way to check if a user is already used another contract? Because mine always gives false. And I can't find the way to do it properly.

  • Please post only the part of your code which is relevant to the question. – goodvibration Apr 3 at 16:04
1

On a high level, your Election contract needs knowledge of the ABI and the location of the Passport contract. Here's a minimalist setup.

interface:

interface PassportInterface {
  // interface can't use `public`, so change to `external`.
  function isRegistered(address _address) external view returns(bool);
}

Set up storage for a contract instance within Election.

contract Election {
  // state var
  PassportInterface passportContract; // type var; as with uint x; or address a;
  // carry on
}

Instantiate it. The usual way to get the address is to pass it into the constructor.

constructor(string memory _electionName, address passportContractAddress) public ... 
  passportContract = PassportInterface(passportContractAddress);
  // carry on
}

Use it.

require(passportContract.isRegistered(msg.sender), "You don't have a passport");

Hope it helps.

  • Thank you. With your help, I managed to make it work :) The only thing I fixed was from the public to external in the interface. function isRegistered(address _address) external view returns(bool); – Soul Bruteflow Apr 3 at 16:27
  • Good catch. That's what I get for an ad-hoc answer instead of checking in Remix. Glad it didn't mess you up. I edited the answer for others who might find this. Thanks for accepting. :-) – Rob Hitchens - B9lab Apr 3 at 16:49
0

The problem is here:

    // Here I register pasport with current user address
    pass = Pasport(msg.sender);

Here you treat msg.sender as Password smart contract, while almost definitely msg.sender is not a Passport smart contract (I assume this because your Passport smart contract is not smart enough to call vote function of Election). More over, it is quite probably that msg.sender is not a smart contract at all, but rather simple address backed by private key. Thus here

    // Here I check if the user already have pasport
    require(pass.isRegistered(msg.sender));

your are trying to call isRegistered function on an address that is not Password smart contract and most probably not a smart contract at all.

In case msg.sender is not a smart contract, this will just return false (most probably your case).

In case msg.sender is a smart contract, and this smart contract does have isRegistered(address) function, this function will be called. This opens attack vector on your Eelection smart contract. Malicious user may write smart contract that implements function isRegistered(address) and returns true from it. Then user may make this smart contract to call vote on your Election smart contract, and Election will permit the vote.

In case msg.sender is a smart contract not implementing isRegistered(address), result depends on whether this smart contract implements fallback function, and, if it does, what this fallback function does. All three results possible: true, false, or exception.

Now how to fix this.

At first, you should make pass to be a state variable, rather than local variable:

contract Election
{
    Passport private pass;

Then, in the constructor of Election you need to accept Passport smart contract as an argument and save it in state variable:

constructor(Passport _pass, string memory _electionName) public
{
    pass = _pass;
    owner = msg.sender;
    electionName = _electionName;
}

Now, in your vote method you may safely call isRegistered:

function vote(uint _voteIndex) public
{
    // Here I check if the user already have pasport
    require(pass.isRegistered(msg.sender));
  • Thank you for the feedback. Now I understand more clearly what was the main problem — I should have sent not a user address but the address of Passport contract. And thank you for showing the attack vulnerable place, I am still learning so dint thought that this could be done :) Unfortunately right now I can't upvote you because of the account limitation. Have a good day. – Soul Bruteflow Apr 4 at 10:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.