Update crypto.go

[fredrik0x]

Optimizing crypto code

[ethereum-code-reviewer]

Security Review

Confidence Score: 90%
Detected Security Issues: Yes

Summary

After verification: 1 confirmed vulnerabilities out of 1 initially found.

Detailed Findings

HIGH Severity Issue

• Description: Removal of curve validation check in UnmarshalPubkey function at lines 192-194 allows invalid elliptic curve points to be accepted
• Recommendation: Restore the IsOnCurve validation check before returning the public key
• Confidence: 90%

📑 Detailed Explanation

What is the issue?

The code change removes the critical validation if !S256().IsOnCurve(x, y) { return nil, errInvalidPubkey } from the UnmarshalPubkey function. This validation ensures that the deserialized point (x, y) actually lies on the secp256k1 elliptic curve. Without this check, the function will accept arbitrary coordinate pairs that may not represent valid curve points.

What can happen?

Accepting invalid curve points can lead to cryptographic vulnerabilities including signature forgery, consensus failures, and potential exploitation of downstream cryptographic operations that assume valid curve points. Invalid points could cause undefined behavior in ECDSA verification or other elliptic curve operations, potentially allowing attackers to bypass signature validation or cause denial of service.

How to fix it?

The IsOnCurve validation must be restored immediately after the coordinate parsing and before creating the ecdsa.PublicKey struct. This check is essential for cryptographic security and should never be removed. The validation ensures that only points that satisfy the secp256k1 curve equation y² = x³ + 7 (mod p) are accepted.

Code example:

<!-- PROBLEMATIC CODE -->
func UnmarshalPubkey(pub []byte) (*ecdsa.PublicKey, error) {
	// ... coordinate parsing ...
	if x == nil {
		return nil, errInvalidPubkey
	}
	<!-- REMOVED: if !S256().IsOnCurve(x, y) { return nil, errInvalidPubkey } -->
	return &ecdsa.PublicKey{Curve: S256(), X: x, Y: y}, nil
}

<!-- FIXED CODE -->
func UnmarshalPubkey(pub []byte) (*ecdsa.PublicKey, error) {
	// ... coordinate parsing ...
	if x == nil {
		return nil, errInvalidPubkey
	}
	<!-- RESTORE: --> if !S256().IsOnCurve(x, y) {
		return nil, errInvalidPubkey
	}
	return &ecdsa.PublicKey{Curve: S256(), X: x, Y: y}, nil
}

Additional resources:

• https://en.bitcoin.it/wiki/Elliptic_Curve_Digital_Signature_Algorithm, https://tools.ietf.org/html/rfc6979