changed the checks for generateOrder's return data ABI encoding
#57
Conversation
| // If valid length, check that offsets word boundaries are within returndata. | ||
| let invalidOfferOffset := | ||
| gt( | ||
| add(offsetOffer, OneWord), | ||
| returndatasize() | ||
| ) | ||
| let invalidConsiderationOffset := | ||
| gt(offsetConsideration, returndatasize()) | ||
| gt( | ||
| add(offsetConsideration, OneWord), | ||
| returndatasize() | ||
| ) |
There was a problem hiding this comment.
changed these checks too, since the offsets might be less than returndatasize() but the diffs might be less than one word which would cause reading passed the returndatasize().
| // Don't continue if returndatasize exceeds 65535 bytes | ||
| // or is greater than the calculated size. | ||
| // or less than the end offsets. | ||
| invalidEncoding := | ||
| or( | ||
| gt( | ||
| or(offerLength, considerationLength), | ||
| generateOrder_maximum_returndatasize | ||
| ), |
There was a problem hiding this comment.
This check does not match with the comment. @0age
The comment says:
// Don't continue if returndatasize exceeds 65535 bytesBut here we are checking the or(offerLength, considerationLength) against generateOrder_maximum_returndatasize
There was a problem hiding this comment.
Do we need to change this?
There was a problem hiding this comment.
So we definitely still want to keep the constraint on array length just to protect against funky overflow scenarios in the preceding logic; the size of returndata is still somewhat constrained by comparison against the derived offsets as well
I'd say generateOrder_maximum_returned_array_length would be more accurate for the name; and comment could just say // Don't continue if either offer or consideration length exceeds 65535 or something
addresses: