[SEC] EIP 2386 Specification Completeness

[eschorn1]

labels: nbc-audit-2020-2 🛂, status:reported
labels: difficulty:medium, severity:medium, type:bug

Description

The EIP 2386 specification is silent on a number of aspects relevant to blscurve/eth2_keygen, including:

• The maximum length of the name string and expectations regarding Unicode normalization.
• The type or maximum allowed nextaccount value.
• The type or maximum expected version value (though this is currently hardcoded and thus not yet necessary).
• Required or expected behavior when extraneous fields are present.

For example, allowing a name string of length one gigabyte is not necessary and may surface downstream impacts stemming from unexpected/unnecessary memory allocations.

While the name string is a simple identifier, different UTF-8 encodings may arise from malicious intent or simply the broad range of participating devices, operating systems, languages and applications. Characters with accents or other modifiers can have multiple correct Unicode encodings. For example, the Á (a-acute) glyph can be encoded as a single character U+00C1 (the "composed" form) or as two separate characters U+0041 then U+0301 (the "decomposed" form). In some cases, the order of a glyph's combining elements is significant and in other cases different orders must be considered equivalent. Normalization is the process of standardizing string representation such that if two strings are canonically equivalent and are normalized to the same normal form, their byte representations will be the same. Only then can string comparison and ordering operations be relied upon. Performing this step is best practice to support user expectations related to rendering consistency.

Regarding nextaccount, values beyond 2**53 are likely not necessary and may encounter problems related to the JavaScript number type having a 53-bit floating-point mantissa. Further, if this value is related to an index in EIP 2333, then a constraint of 2**32 is more reasonable.

Specifying similar constraints for version can be done alongside other modifications for completeness, though this is not currently necessary.

Specifying required or expected behavior when extraneous fields are present will improve implementation interoperability.

Mitigation Recommendation

To summarize, it is recommended to consider a maximum string length for name and to indicate that implementations should immediately normalize this value to the NKFC form per section 2.11.2.B.2 of Unicode Technical Report #36. Additionally, consider specifying the type and maximum values for both the nextaccount and version values, as well as what should happen if extraneous fields are present.

This issue is also submitted to the EIP 2386 discussion at ethereum/EIPs#2386.

Both nim-blscurve and nim-beacon-chain should track the resolution and implement the corresponding validation checks.