phoreproject / bls

Go implementation of the BLS12-381 pairing
Apache License 2.0
89 stars 31 forks source link

DecompressGn should handle invalid data without panic #12

Closed Nanyan closed 4 years ago

Nanyan commented 5 years ago

Hi there. It's a great job to implementing BLS in pure go. I've made some test on it, and encountered a panic when calling DecompressG1 with an invalid data, which is 83051c87397e54313c98ad614e3f2085e43cd2c8cb5262c8d6cc27c871b91efabbbfd13033938e8bb95fdb3da5973dfd in the form of hex string.

I debug into it, and found the cause(corresponding to commit fb0e03c):

// DecompressG1 decompresses the big int into an affine point and checks
// if it is in the correct prime group.
func DecompressG1(b [48]byte) (*G1Affine, error) {
    affine, err := DecompressG1Unchecked(b)
    if err != nil {
        return nil, err
    }

    if !affine.IsInCorrectSubgroupAssumingOnCurve() {       //g1.go line191: panic here
        return nil, errors.New("not in correct subgroup")
    }         
    return affine, nil
}

// DecompressG1Unchecked decompresses the big int into an affine point without
// checking if it's in the correct prime group.
func DecompressG1Unchecked(b [48]byte) (*G1Affine, error) {
    var copyBytes [48]byte
    copy(copyBytes[:], b[:])

    if len(copyBytes) == 0 || copyBytes[0]&(1<<7) == 0 {
        return nil, errors.New("unexpected compression mode")
    }

    if copyBytes[0]&(1<<6) != 0 {
        // this is the point at infinity
        copyBytes[0] &= 0x3f

        for _, b := range copyBytes {
            if b != 0 {
                return nil, errors.New("unexpected information in compressed infinity")
            }
        }

        return G1AffineZero.Copy(), nil
    }
    greatest := copyBytes[0]&(1<<5) != 0

    copyBytes[0] &= 0x1f

    x := FQReprFromBytes(copyBytes)
    xFQ := FQReprToFQ(x)

    return GetG1PointFromX(xFQ, greatest), nil   //g1.go line226: here will return nil,nil then cause panic
}

func GetG1PointFromX(x FQ, greatest bool) *G1Affine {
    x3b := x.Copy()
    x3b.SquareAssign()
    x3b.MulAssign(x)
    x3b.AddAssign(BCoeff)

    y, success := x3b.Sqrt()

    if !success {
        return nil           //g1.go line120: here‘s the root cause!
    }

    negY := y.Copy()
    negY.NegAssign()

    yVal := negY
    if (y.Cmp(negY) < 0) != greatest {
        yVal = y
    }
    return NewG1Affine(x, yVal)
}
meyer9 commented 5 years ago

This is because the point is not on the curve (x^3 + b = y^2). It would still be good to handle that error properly.

protolambda commented 4 years ago

Stumbled on the same panic by accident, here's another test case: https://github.com/protolambda/bls/commit/c0bdc9baa52fc56a10ef258fe6e17cf0bfc035c1