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
NamecheapPushDomainVerifier
template currently validatesfromEmailIndex < emailHeaderLength
,namecheapBuyerIdIndex < emailBodyLength
, andnamecheapDomainNameIndex < emailBodyLength
withLessThan
template.However, the$N$ ,
LessThan(N)
template has a known over-flow issue: If the bit-length of the input exceedsLessThan
may 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
Num2Bits
fromcircomlib
.Reference
For more details on this vulnerability, refer to:
Note
FromRegex
,BodyHashRegex
, andVenmoTimestampRegex
also containsLessThan
without bit-length check. However, this seems fine since they are used for utf-8 body.