Create _readSignatureType with minimal validation

This commit is contained in:
Amir Bandeali 2019-09-02 20:34:23 -07:00
parent 48dfb3317a
commit a6b60f3230

View File

@ -186,11 +186,15 @@ contract MixinSignatureValidator is
pure pure
returns (bool needsRegularValidation) returns (bool needsRegularValidation)
{ {
SignatureType signatureType = _readValidSignatureType( // Read the signatureType from the signature
SignatureType signatureType = _readSignatureType(
hash, hash,
signerAddress, signerAddress,
signature signature
); );
// Any signature type that makes an external call needs to be revalidated
// with every partial fill
needsRegularValidation = needsRegularValidation =
signatureType == SignatureType.Wallet || signatureType == SignatureType.Wallet ||
signatureType == SignatureType.Validator || signatureType == SignatureType.Validator ||
@ -396,7 +400,28 @@ contract MixinSignatureValidator is
} }
} }
/// Reads the `SignatureType` from the end of a signature and validates it. /// @dev Reads the `SignatureType` from a signature with minimal validation.
function _readSignatureType(
bytes32 hash,
address signerAddress,
bytes memory signature
)
private
pure
returns (SignatureType signatureType)
{
if (signature.length == 0) {
LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError(
LibExchangeRichErrors.SignatureErrorCodes.INVALID_LENGTH,
hash,
signerAddress,
signature
));
}
return SignatureType(uint8(signature[signature.length - 1]));
}
/// @dev Reads the `SignatureType` from the end of a signature and validates it.
function _readValidSignatureType( function _readValidSignatureType(
bytes32 hash, bytes32 hash,
address signerAddress, address signerAddress,
@ -406,6 +431,13 @@ contract MixinSignatureValidator is
pure pure
returns (SignatureType signatureType) returns (SignatureType signatureType)
{ {
// Read the signatureType from the signature
SignatureType signatureType = _readSignatureType(
hash,
signerAddress,
signature
);
// Disallow address zero because it is ecrecover() returns zero on // Disallow address zero because it is ecrecover() returns zero on
// failure. // failure.
if (signerAddress == address(0)) { if (signerAddress == address(0)) {
@ -417,20 +449,8 @@ contract MixinSignatureValidator is
)); ));
} }
if (signature.length == 0) {
LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError(
LibExchangeRichErrors.SignatureErrorCodes.INVALID_LENGTH,
hash,
signerAddress,
signature
));
}
// Read the last byte off of signature byte array.
uint8 signatureTypeRaw = uint8(signature[signature.length - 1]);
// Ensure signature is supported // Ensure signature is supported
if (signatureTypeRaw >= uint8(SignatureType.NSignatureTypes)) { if (uint8(signatureType) >= uint8(SignatureType.NSignatureTypes)) {
LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError( LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError(
LibExchangeRichErrors.SignatureErrorCodes.UNSUPPORTED, LibExchangeRichErrors.SignatureErrorCodes.UNSUPPORTED,
hash, hash,
@ -444,7 +464,7 @@ contract MixinSignatureValidator is
// signature array with invalid type or length. We may as well make // signature array with invalid type or length. We may as well make
// it an explicit option. This aids testing and analysis. It is // it an explicit option. This aids testing and analysis. It is
// also the initialization value for the enum type. // also the initialization value for the enum type.
if (SignatureType(signatureTypeRaw) == SignatureType.Illegal) { if (signatureType == SignatureType.Illegal) {
LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError( LibRichErrors.rrevert(LibExchangeRichErrors.SignatureError(
LibExchangeRichErrors.SignatureErrorCodes.ILLEGAL, LibExchangeRichErrors.SignatureErrorCodes.ILLEGAL,
hash, hash,
@ -453,7 +473,7 @@ contract MixinSignatureValidator is
)); ));
} }
return SignatureType(signatureTypeRaw); return signatureType;
} }
/// @dev Verifies a hash and signature using logic defined by Wallet contract. /// @dev Verifies a hash and signature using logic defined by Wallet contract.