Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Add Fuzzing to VRF #1512

Closed
wants to merge 29 commits into from
Closed

Add Fuzzing to VRF #1512

wants to merge 29 commits into from

Conversation

gdbelvin
Copy link
Contributor

@gdbelvin gdbelvin commented Apr 7, 2020

Fuzzing did, in fact, find a bug :-)

Depends on #1346

@dep dep bot added the dependent label Apr 7, 2020
//
// http://www.secg.org/sec1-v2.pdf
// https://tools.ietf.org/html/rfc8032#section-5.1.3
func SECG1Decode(curve elliptic.Curve, data []byte) (x, y *big.Int) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

77-106 lines are duplicate of core/crypto/vrf/p256/unmarshal.go:27-56 (from dupl)

}
}

func BenchmarkProveECVRFP256SHA256SWUU(b *testing.B) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

197-218 lines are duplicate of core/crypto/draft-irtf-cfrg-vrf-06/ecvrf_p256_sha256_tai_test.go:197-218 (from dupl)

}
}

func BenchmarkProveECVRFP256SHA256TAI(b *testing.B) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

197-218 lines are duplicate of core/crypto/draft-irtf-cfrg-vrf-06/ecvrf_p256_sha256_swu_test.go:197-218 (from dupl)

if aux.params.cofactor > 1 {
hx, hy = aux.params.ec.ScalarMult(hx, hy, []byte{aux.params.cofactor})
}
return

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naked return in func hashToCurveSimplifiedSWU with 98 lines of code (from nakedret)

@gdbelvin gdbelvin force-pushed the vrf-fuzz branch 2 times, most recently from 487f8d5 to db8e3f9 Compare April 7, 2020 11:11
@codecov
Copy link

codecov bot commented Apr 7, 2020

Codecov Report

Merging #1512 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1512   +/-   ##
=======================================
  Coverage   68.79%   68.79%           
=======================================
  Files          55       55           
  Lines        4063     4063           
=======================================
  Hits         2795     2795           
  Misses        862      862           
  Partials      406      406           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6115a89...e1e984c. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 3 alerts when merging db8e3f9 into 81611a7 - view on LGTM.com

new alerts:

  • 3 for Useless assignment to local variable

return generateNonceRFC6979(a.params.hash, sk, h)
}

<<<<<<< HEAD

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected declaration, found '<<' (from typecheck)


// 5. While H is "INVALID" or H is EC point at infinity:
var err error
for Hx == nil || err != nil || (zero.Cmp(Hx) == 0 && zero.Cmp(Hy) == 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected declaration, found 'for' (from typecheck)

"math/big"
)

var zero big.Int

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zero redeclared in this block (from typecheck)

"math/big"
)

var zero big.Int

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other declaration of zero (from `typecheck`)

// Output:
// - `H` - hashed value, a finite EC point in G
// - `ctr` - integer, number of suite byte, attempts to find a valid curve point
func HashToCurveTryAndIncrement(v *ECVRFSuite, Y *PublicKey, alpha []byte) (Hx, Hy *big.Int, ctr uint) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HashToCurveTryAndIncrement redeclared in this block (from typecheck)

}
m := []byte("sample")

if got, want := GenerateNonceRFC6979(hash, SK, m).Bytes(),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undeclared name: GenerateNonceRFC6979 (from typecheck)

// hLen - output length in octets of Hash; must be at least 2n
Hash: crypto.SHA256,
Conversions: Conversions{
Int2String: I2OSP, // RFC8017 section-4.1 (big endian representation)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot use I2OSP (value of type func(x *big.Int, rLen uint) []byte) as func(a uint, l uint) []byte value in struct literal (from typecheck)

String2Point: SECG1Decode, // Section 2.3.4 of [SECG1]

// ArbitraryString2Point returns string_to_point(0x02 || h_string)
ArbitraryString2Point: ArbitraryString2Point,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undeclared name: ArbitraryString2Point (from typecheck)

// 12. H_prelim = arbitrary_string_to_point(int_to_string(final_x, 2n))
// (note: arbitrary_string_to_point will not return INVALID by
// correctness of Simple SWU)
hx, hy, _ = aux.ArbitraryStringToPoint(aux.IntToString(xFinal, uint(aux.params.n*2)))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot use xFinal (variable of type *big.Int) as uint value in argument to aux.IntToString (from typecheck)

cofactor: 1,
hash: crypto.SHA256,
}
p256SHA256SWU.ECVRFParams.aux = p256SHA256SWUAux{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot use (p256SHA256SWUAux literal) (value of type p256SHA256SWUAux) as ECVRFAux value in assignment: wrong type for method IntToString (from typecheck)

@lgtm-com
Copy link

lgtm-com bot commented Apr 20, 2020

This pull request introduces 3 alerts when merging e1e984c into 6115a89 - view on LGTM.com

new alerts:

  • 3 for Useless assignment to local variable

@gdbelvin gdbelvin closed this Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants