Fix Potential Overflow Problem within NamecheapPushDomainVerifier
#423
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
NamecheapPushDomainVerifiertemplate currently validatesfromEmailIndex < emailHeaderLength,namecheapBuyerIdIndex < emailBodyLength, andnamecheapDomainNameIndex < emailBodyLengthwithLessThantemplate.However, the$N$ ,
LessThan(N)template has a known over-flow issue: If the bit-length of the input exceedsLessThanmay produce unintended results. For example, supposemaxHeadersLength = 768,emailHeaderLength = 512, andfromEmailIndex = 21888242871839275222246405745257275088548364400416034343698204186575808495588. In this case, the output ofLessThan(log2Ceil(maxHeadersLength))([fromEmailIndex, emailHeaderLength])is1, meaning that this malicious input satisfies the constraints ofNamecheapPushDomainVerifier.Fix
To address this problem, I implemented a check on the bit-length of inputs with
Num2Bitsfromcircomlib.Reference
For more details on this vulnerability, refer to:
Note
FromRegex,BodyHashRegex, andVenmoTimestampRegexalso containsLessThanwithout bit-length check. However, this seems fine since they are used for utf-8 body.