A bad implementation of the algorithm ecAdd was found. In this implementation, all cases and exceptions are not covered. This will produce undesired results, which can result in unhandled errors and/or denial of services in some circumstances.
Impact
During the audit, it has been observed that when making sums between two points in the ElipticCurve.sol library, the value of Y is not checked. Therefore, when the value of X1 and X2 is the same, the value of Y should be also verified, otherwise it could lead to an infinite point.
As we can see in the following examples taken from other known implementations (BouncyCastle, ANSSI-FR), the value of the Y is always checked in order to return an infinity point or not.
Finding discovered during Red4Sec security audit.
Description
A bad implementation of the algorithm ecAdd was found. In this implementation, all cases and exceptions are not covered. This will produce undesired results, which can result in unhandled errors and/or denial of services in some circumstances.
Impact
During the audit, it has been observed that when making sums between two points in the ElipticCurve.sol library, the value of Y is not checked. Therefore, when the value of X1 and X2 is the same, the value of Y should be also verified, otherwise it could lead to an infinite point.
As we can see in the following examples taken from other known implementations (BouncyCastle, ANSSI-FR), the value of the Y is always checked in order to return an infinity point or not.
Code References:
https://github.com/witnet/elliptic-curve-solidity/blob/7d814018d460849deabc882ef19c254160e06301/contracts/EllipticCurve.sol#L176
Mitigations