Merge pull request #16 from colinnielsen/colinnielsen/vulnerability

Critical Vulnerability

.github/workflows/noir.yml

@@ -19,7 +19,7 @@ jobs:
       - name: Install Nargo
         uses: noir-lang/[email protected]
         with:
-          toolchain: v0.19.0
+          toolchain: v0.36.0
 
       - name: Run nargo test
         run: |

Nargo.toml

@@ -1,9 +1,9 @@
 [package]
 authors = ["@colinnielsen"]
-compiler_version = ">=0.19.0"
+compiler_version = ">=0.30.0"
 name = "ecrecover"
 notes = "AMDG"
 type = "lib"
 
 [dependencies]
-array_helpers = { tag = "v0.19.0", git = "https://github.com/colinnielsen/noir-array-helpers" }
+array_helpers = { tag = "v0.30.0", git = "https://github.com/colinnielsen/noir-array-helpers" }

README.md

@@ -5,6 +5,14 @@
 
 **This software is unaudited and should not be used in production. Use at your own risk.**
 
+## WARNING!
+
+**DO NOT use versions of this library < `v0.30.0`**
+
+This library contained a critical vulnerability found by @olehmisar. The circuits were critically underconstrained, allowing anyone to impersonate public Ethereum addresses.
+
+See more details [here](https://gist.github.com/olehmisar/4cfe6128eaac2bfbe1fa8eb46f0116d6).
+
 <hr>
 
 **ECRecover Noir** includes tools to help prove secp256k1 signatures (Ethereum's curve) in Noir Circuits.
@@ -21,7 +29,7 @@ In your `Nargo.toml` file, add the following dependency:
 
 ```toml
 [dependencies]
-ecrecover = { tag = "v0.19.0", git = "https://github.com/colinnielsen/ecrecover-noir" }
+ecrecover = { tag = "v0.30.0", git = "https://github.com/colinnielsen/ecrecover-noir" }
 ```
 
 ## Simple Usage

src/lib.nr

@@ -1,8 +1,6 @@
-use dep::std;
-
 mod secp256k1;
 
-fn ecrecover(
+pub fn ecrecover(
     pub_key_x: [u8; 32],
     pub_key_y: [u8; 32],
     signature: [u8; 64], // clip v value

src/secp256k1.nr

@@ -1,13 +1,11 @@
 use dep::std;
-
 use dep::array_helpers;
 
-struct PubKey {
+pub struct PubKey {
     pub_x: [u8; 32],
     pub_y: [u8; 32],
 }
 
-unconstrained
 fn split_uncompressed_pub_key(
     pub_key: [u8; 65]
 ) -> ([u8; 32], [u8; 32]) {
@@ -23,23 +21,23 @@ fn split_uncompressed_pub_key(
 }
 
 impl PubKey {
-    fn from_xy(pub_x: [u8; 32], pub_y: [u8; 32]) -> PubKey {
+    pub fn from_xy(pub_x: [u8; 32], pub_y: [u8; 32]) -> PubKey {
         PubKey {
             pub_x,
             pub_y,
         }
     }
 
-    fn from_unified(pub_key: [u8; 64]) -> PubKey {
-        let (key_x, key_y) = array_helpers::split_u8_64_unconstrained(pub_key);
+    pub fn from_unified(pub_key: [u8; 64]) -> PubKey {
+        let (key_x, key_y) = array_helpers::split_u8_64(pub_key);
 
         PubKey {
             pub_x: key_x,
             pub_y: key_y,
         }
     }
 
-    fn from_uncompressed(pub_key: [u8; 65]) -> PubKey {
+    pub fn from_uncompressed(pub_key: [u8; 65]) -> PubKey {
         assert(pub_key[0] == 0x04);
         let (key_x, key_y) = split_uncompressed_pub_key(pub_key);
 
@@ -49,12 +47,12 @@ impl PubKey {
         }
     }
 
-    fn verify_sig(self, signature: [u8; 64], hashed_message: [u8; 32]) -> bool {
+    pub fn verify_sig(self, signature: [u8; 64], hashed_message: [u8; 32]) -> bool {
         std::ecdsa_secp256k1::verify_signature(self.pub_x, self.pub_y, signature, hashed_message)
     }
 
-    fn to_eth_address(self) -> Field {
-        let pub_key = array_helpers::u8_32_to_u8_64_unconstrained(self.pub_x, self.pub_y);
+    pub fn to_eth_address(self) -> Field {
+        let pub_key = array_helpers::u8_32_to_u8_64(self.pub_x, self.pub_y);
         let hashed_pub_key = std::hash::keccak256(pub_key, 64);
 
         let mut addr: Field = 0;