status-im / nim-blscurve

Nim implementation of BLS signature scheme (Boneh-Lynn-Shacham) over Barreto-Lynn-Scott (BLS) curve BLS12-381
Apache License 2.0
26 stars 11 forks source link

[SEC] Missing `norm` Operations #75

Closed eschorn1 closed 2 years ago

eschorn1 commented 4 years ago

labels: nbc-audit-2020-1, status:reported labels: difficulty:high, severity:medium, type:bug

Description

The isogeny_map_G2() function excerpted below as implemented in hash_to_curve.nim implements the 3-isogeny map from a point P' (x', y') on G'2 to a point P(x, y) on the G2 curve of BLS12-381. The bulk of the logic relates to similar calculations of xNum, xDen, yNum and yDen with the first shown below.

# xNum = k(1,3) * x'³ + k(1,2) * x'² + k(1,1) * x' + k(1,0)
let xNum = block:
  var xNum = k13.mul(xp3)
  norm(xNum)
  xNum.add xNum, k12.mul(xp2)
  norm(xNum)
  xNum.add xNum, k11.mul(xp)
  norm(xNum)
  xNum.add xNum, k10
  xNum

For 58-bit limbs, the add function ultimately resolves to BIG_384_58_add() in the csources, which does not reduce its result. Thus, the final result is missing a norm() operation prior to its return. This can impact correct operation as the xNum and yNum values subsequently become operands in a multiplication where the comments in the FP2_BLS381_mul() execution path state that inputs must be normed. A similar scenario applies to the case of 29-bit limbs.

Exploit Scenario

The logic may calculate incorrect results that will be extremely difficult to debug.

Mitigation Recommendation

Add a final norm to the calculation of xNum, xDen, yNum and yDen.

References

  1. https://github.com/status-im/nim-blscurve/blob/da9ae49dab4cbb95b2cdaa68ecc221ee15c67a9a/blscurve/miracl/hash_to_curve.nim#L374