From 8e7aef083e18d3f63a127be89083fdcd0b2adf2e Mon Sep 17 00:00:00 2001
From: Jonas Nick <jonasd.nick@gmail.com>
Date: Mon, 4 Nov 2019 12:53:37 +0000
Subject: [PATCH 1/2] Fix point_from_bytes accepting out-of-range pubkeys and
 add test vector

---
 bip-schnorr/reference.py    |  2 ++
 bip-schnorr/test-vectors.py | 26 +++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/bip-schnorr/reference.py b/bip-schnorr/reference.py
index 79be6903..f2a944f1 100644
--- a/bip-schnorr/reference.py
+++ b/bip-schnorr/reference.py
@@ -53,6 +53,8 @@ def bytes_from_point(P):
 
 def point_from_bytes(b):
     x = int_from_bytes(b)
+    if x >= p:
+        return None
     y_sq = (pow(x, 3, p) + 7) % p
     y = pow(y_sq, (p + 1) // 4, p)
     if pow(y, 2, p) != y_sq:
diff --git a/bip-schnorr/test-vectors.py b/bip-schnorr/test-vectors.py
index da7aec66..195b61b2 100644
--- a/bip-schnorr/test-vectors.py
+++ b/bip-schnorr/test-vectors.py
@@ -69,12 +69,14 @@ def vector4():
 default_seckey = bytes_from_int(0xB7E151628AED2A6ABF7158809CF4F3C762E7160F38B4DA56A784D9045190CFEF)
 default_msg = bytes_from_int(0x243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89)
 
+# Public key is not on the curve
 def vector5():
+    # This creates a dummy signature that doesn't have anything to do with the
+    # public key.
     seckey = default_seckey
     msg = default_msg
     sig = schnorr_sign(msg, seckey)
 
-    # Public key is not on the curve
     pubkey = bytes_from_int(0xEEFDEA4CDB677750A420FEE807EACF21EB9898AE79B9768766E4FAA04A2D4A34)
     assert(point_from_bytes(pubkey) is None)
 
@@ -185,6 +187,27 @@ def vector13():
 
     return (None, pubkey_gen(seckey), msg, sig, "FALSE", "sig[32:64] is equal to curve order")
 
+# Test out of range pubkey
+# It's cryptographically impossible to create a test vector that fails if run
+# in an implementation which accepts out of range pubkeys because we can't find
+# a secret key for such a public key and therefore can not create a signature.
+# This test vector just increases test coverage.
+def vector14():
+    # This creates a dummy signature that doesn't have anything to do with the
+    # public key.
+    seckey = default_seckey
+    msg = default_msg
+    sig = schnorr_sign(msg, seckey)
+
+    pubkey_int = p + 1
+    pubkey = bytes_from_int(pubkey_int)
+    assert(point_from_bytes(pubkey) is None)
+    # If an implementation would reduce a given public key modulo p then the
+    # pubkey would be valid
+    assert(point_from_bytes(bytes_from_int(pubkey_int % p)) is not None)
+
+    return (None, pubkey, msg, sig, "FALSE", "public key is not a valid X coordinate because it exceeds the field size")
+
 vectors = [
         vector0(),
         vector1(),
@@ -200,6 +223,7 @@ vectors = [
         vector11(),
         vector12(),
         vector13(),
+        vector14()
     ]
 
 # Converts the byte strings of a test vector into hex strings

From 8a8a35bfc58e5d1a8bf78d0b3da486a8dcbd49f9 Mon Sep 17 00:00:00 2001
From: Jonas Nick <jonasd.nick@gmail.com>
Date: Tue, 5 Nov 2019 10:14:23 +0000
Subject: [PATCH 2/2] Update test-vectors.csv

---
 bip-schnorr/test-vectors.csv | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bip-schnorr/test-vectors.csv b/bip-schnorr/test-vectors.csv
index 9cd0d831..39708036 100644
--- a/bip-schnorr/test-vectors.csv
+++ b/bip-schnorr/test-vectors.csv
@@ -13,3 +13,4 @@ index,secret key,public key,message,signature,verification result,comment
 11,,DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,4A298DACAE57395A15D0795DDBFD1DCB564DA82B0F269BC70A74F8220429BA1D4160BCFC3F466ECB8FACD19ADE57D8699D74E7207D78C6AEDC3799B52A8E0598,FALSE,sig[0:32] is not an X coordinate on the curve
 12,,DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F4160BCFC3F466ECB8FACD19ADE57D8699D74E7207D78C6AEDC3799B52A8E0598,FALSE,sig[0:32] is equal to field size
 13,,DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,667C2F778E0616E611BD0C14B8A600C5884551701A949EF0EBFD72D452D64E84FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141,FALSE,sig[32:64] is equal to curve order
+14,,FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,667C2F778E0616E611BD0C14B8A600C5884551701A949EF0EBFD72D452D64E844160BCFC3F466ECB8FACD19ADE57D8699D74E7207D78C6AEDC3799B52A8E0598,FALSE,public key is not a valid X coordinate because it exceeds the field size