Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 4 #4

Open
Edward-Lo opened this issue Sep 10, 2020 · 0 comments
Open

Issue 4 #4

Edward-Lo opened this issue Sep 10, 2020 · 0 comments

Comments

@Edward-Lo
Copy link
Collaborator

Description

There is a misused sanity check in the TransferFrom method of flamingo staking contract. Owner can spend the tokens approved to be spent by spender.

Function TransferFrom can be called by the spender to spend the tokens approved by the owner.

Staking/flamingo-contract-staking/FLM/FLM.cs

public static bool TransferFrom(byte[] spender, byte[] owner, byte[] receiver, BigInteger amt, byte[] callingScript)
{
    assert(spender.Length == 20 && owner.Length == 20 && receiver.Length == 20, "transferFrom: invalid spender or owner or receiver, spender-".AsByteArray().Concat(spender).Concat(", owner-".AsByteArray()).Concat(owner).Concat(" and receiver-".AsByteArray()).Concat(receiver).AsString());
    assert(amt > 0, "transferFrom: invalid amount-".AsByteArray().Concat(amt.ToByteArray()).AsString());
    assert(Runtime.CheckWitness(spender) || owner.Equals(callingScript), "transferFrom: CheckWitness failed, spender-".AsByteArray().Concat(spender).AsString());

    if (spender.Equals(owner) || owner.Equals(receiver))
    {
        return true;
    }
    ......
}

However, if the owner calls this function, owner.Equals(callingScript) will be true and the owner is able to spend the tokens that are approved to be spent by the spender.

Recommendation

Owner shouldn't be able to call the TransferFrom function to spend the tokens.

Staking/flamingo-contract-staking/FLM/FLM.cs

public static bool TransferFrom(byte[] spender, byte[] owner, byte[] receiver, BigInteger amt, byte[] callingScript)
{
    assert(spender.Length == 20 && owner.Length == 20 && receiver.Length == 20, "transferFrom: invalid spender or owner or receiver, spender-".AsByteArray().Concat(spender).Concat(", owner-".AsByteArray()).Concat(owner).Concat(" and receiver-".AsByteArray()).Concat(receiver).AsString());
    assert(amt > 0, "transferFrom: invalid amount-".AsByteArray().Concat(amt.ToByteArray()).AsString());
    assert(Runtime.CheckWitness(spender), "transferFrom: CheckWitness failed, spender-".AsByteArray().Concat(spender).AsString());

    if (spender.Equals(owner) || owner.Equals(receiver))
    {
        return true;
    }
    ......
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant