[Bug] checkSigs returns true on error

[bh2smith]

Problem

When calling Validate.checkSigs on hashes and signatures that do not "line up", the ECRecovery returns a value of zero and hence the equality at line 26 and 29 yields a value of true. This is certainly undesired and appears to be a security flaw in the transaction exit procedure of plasma.

Observe;

https://github.com/omisego/plasma-mvp/blob/18df4c1812b4da004f4ca19bce08121f96ecf59d/plasma/root_chain/contracts/Validate.sol#L26-L33

Example

The following call with obviously incorrect parameters returns true;

checkSigs.call(zeroHash32, zeroHash32, 0, zeroSig195)

Why does this happen

This is a result of ECRecovery's use of the solidity function "ecrecover"

ecrecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) returns (address): 
    recover address associated with the public key from elliptic curve signature, return zero on error

Proposed Solution

Check that one of the two items in the declaration of check1 and check2 is non-zero && the other comparison. For example,

address txHashSig1 = ECRecovery.recover(txHash, sig1);
check1 = txHashSig1 != 0 && txHashSig1 == ECRecovery.recover(confirmationHash, confSig1);