Fix trailing null bytes in JSON output and add C-string termination

.github/workflows/build.yml

@@ -264,7 +264,7 @@ jobs:
           make ios
           make ios_simulator
           make macos_arm64
-          ctest --rerun-failed --output-on-failure
+          cd build_prover_macos_arm64 && ctest --rerun-failed --output-on-failure
 
       - name: test rapidsnark
         run: |

.vscode/settings.json

@@ -71,6 +71,12 @@
         "csignal": "cpp",
         "set": "cpp",
         "valarray": "cpp",
-        "variant": "cpp"
+        "variant": "cpp",
+        "__verbose_abort": "cpp",
+        "execution": "cpp",
+        "__node_handle": "cpp",
+        "__tree": "cpp",
+        "ios": "cpp",
+        "locale": "cpp"
     }
 }

build_gmp.sh

@@ -27,16 +27,36 @@ get_gmp()
 {
     GMP_NAME=gmp-6.2.1
     GMP_ARCHIVE=${GMP_NAME}.tar.xz
-    GMP_URL=https://ftp.gnu.org/gnu/gmp/${GMP_ARCHIVE}
+    GMP_MIRRORS=(
+        "https://ftpmirror.gnu.org/gmp/${GMP_ARCHIVE}"
+        "https://gmplib.org/download/gmp/${GMP_ARCHIVE}"
+        "https://ftp.gnu.org/gnu/gmp/${GMP_ARCHIVE}"
+    )
 
     if [ ! -f ${GMP_ARCHIVE} ]; then
-
-        $fetch_cmd ${GMP_URL}
+        for url in "${GMP_MIRRORS[@]}"; do
+            echo "Attempting to download from: $url"
+            set +e
+            $fetch_cmd "$url"
+            exit_code=$?
+            set -e
+
+            if [ $exit_code -eq 0 ]; then
+                echo "Successfully downloaded from: $url"
+                break
+            else
+                echo "Failed to download from: $url"
+                rm -f ${GMP_ARCHIVE}
+            fi
+        done
+
+        if [ ! -f ${GMP_ARCHIVE} ]; then
+            echo "ERROR: Failed to download GMP from all mirrors"
+            exit 1
+        fi
     fi
 
-
     if [ ! -d gmp ]; then
-
         tar -xvf ${GMP_ARCHIVE}
         mv ${GMP_NAME} gmp
     fi

src/prover.cpp

@@ -1,6 +1,7 @@
 #include <gmp.h>
 #include <string>
 #include <cstring>
+#include <cstdarg>
 #include <stdexcept>
 #include <alt_bn128.hpp>
 #include <nlohmann/json.hpp>
@@ -14,13 +15,6 @@
 using json = nlohmann::json;
 
 
-class ShortBufferException : public std::invalid_argument
-{
-public:
-    explicit ShortBufferException(const std::string &msg)
-        : std::invalid_argument(msg) {}
-};
-
 class InvalidWitnessLengthException : public std::invalid_argument
 {
 public:
@@ -31,23 +25,33 @@ class InvalidWitnessLengthException : public std::invalid_argument
 static void
 CopyError(
     char                 *error_msg,
-    unsigned long long    error_msg_maxsize,
+    size_t                error_msg_maxsize,
     const std::exception &e)
 {
-    if (error_msg) {
-        strncpy(error_msg, e.what(), error_msg_maxsize);
-    }
+    if (!error_msg || error_msg_maxsize == 0) return;
+    std::snprintf(error_msg, error_msg_maxsize, "%s", e.what());
 }
 
 static void
-CopyError(
+CopyErrorFmt(
     char               *error_msg,
     unsigned long long  error_msg_maxsize,
-    const char         *str)
+    const char         *format,
+    ...) __attribute__((format(printf, 3, 4)));
+
+static void
+CopyErrorFmt(
+    char               *error_msg,
+    unsigned long long  error_msg_maxsize,
+    const char         *format,
+    ...)
 {
-    if (error_msg) {
-        strncpy(error_msg, str, error_msg_maxsize);
-    }
+    if (!error_msg || error_msg_maxsize == 0) return;
+
+    va_list args;
+    va_start(args, format);
+    std::vsnprintf(error_msg, error_msg_maxsize, format, args);
+    va_end(args);
 }
 
 static unsigned long long
@@ -90,33 +94,6 @@ BuildPublicString(AltBn128::FrElement *wtnsData, uint32_t nPublic)
     return jsonPublic.dump();
 }
 
-static void
-CheckAndUpdateBufferSizes(
-    unsigned long long   proofCalcSize,
-    unsigned long long  *proofSize,
-    unsigned long long   publicCalcSize,
-    unsigned long long  *publicSize,
-    const std::string   &type)
-{
-    if (*proofSize < proofCalcSize || *publicSize < publicCalcSize) {
-
-        *proofSize  = proofCalcSize;
-        *publicSize = publicCalcSize;
-
-        if (*proofSize < proofCalcSize) {
-            throw ShortBufferException("Proof buffer is too short. " + type + " size: "
-                                       + std::to_string(proofCalcSize) +
-                                       ", actual size: "
-                                       + std::to_string(*proofSize));
-        } else {
-            throw ShortBufferException("Public buffer is too short. " + type + " size: "
-                                       + std::to_string(proofCalcSize) +
-                                       ", actual size: "
-                                       + std::to_string(*proofSize));
-       }
-    }
-}
-
 class Groth16Prover
 {
     BinFileUtils::BinFile zkey;
@@ -210,7 +187,7 @@ groth16_public_size_for_zkey_buf(
         return PROVER_ERROR;
 
     } catch (...) {
-        CopyError(error_msg, error_msg_maxsize, "unknown error");
+        CopyErrorFmt(error_msg, error_msg_maxsize, "unknown error");
         return PROVER_ERROR;
     }
 
@@ -235,7 +212,7 @@ groth16_public_size_for_zkey_file(
         return PROVER_ERROR;
 
     } catch (...) {
-        CopyError(error_msg, error_msg_maxsize, "unknown error");
+        CopyErrorFmt(error_msg, error_msg_maxsize, "unknown error");
         return PROVER_ERROR;
     }
 
@@ -280,7 +257,7 @@ groth16_prover_create(
         return PROVER_ERROR;
 
     } catch (...) {
-        CopyError(error_msg, error_msg_maxsize, "unknown error");
+        CopyErrorFmt(error_msg, error_msg_maxsize, "unknown error");
         return PROVER_ERROR;
     }
 
@@ -324,60 +301,63 @@ groth16_prover_prove(
     char                *error_msg,
     unsigned long long   error_msg_maxsize)
 {
-    try {
-        if (prover_object == NULL) {
-            throw std::invalid_argument("Null prover object");
-        }
-
-        if (wtns_buffer == NULL) {
-            throw std::invalid_argument("Null witness buffer");
-        }
+    if (!prover_object) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null prover object");
+        return PROVER_ERROR;
+    }
 
-        if (proof_buffer == NULL) {
-            throw std::invalid_argument("Null proof buffer");
-        }
+    if (!wtns_buffer) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null witness buffer");
+        return PROVER_ERROR;
+    }
 
-        if (proof_size == NULL) {
-            throw std::invalid_argument("Null proof size");
-        }
+    if (!proof_buffer) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null proof buffer");
+        return PROVER_ERROR;
+    }
 
-        if (public_buffer == NULL) {
-            throw std::invalid_argument("Null public buffer");
-        }
+    if (!proof_size) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null proof size");
+        return PROVER_ERROR;
+    }
 
-        if (public_size == NULL) {
-            throw std::invalid_argument("Null public size");
-        }
+    if (!public_buffer) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null public buffer");
+        return PROVER_ERROR;
+    }
 
-        Groth16Prover *prover = static_cast<Groth16Prover*>(prover_object);
+    if (!public_size) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Null public size");
+        return PROVER_ERROR;
+    }
 
-        CheckAndUpdateBufferSizes(prover->proofBufferMinSize(), proof_size,
-                                  prover->publicBufferMinSize(), public_size,
-                                  "Minimum");
+    Groth16Prover *prover = static_cast<Groth16Prover*>(prover_object);
 
-        std::string stringProof;
-        std::string stringPublic;
+    unsigned long long minProofSize = prover->proofBufferMinSize();
+    unsigned long long minPublicSize = prover->publicBufferMinSize();
 
-        prover->prove(wtns_buffer, wtns_size, stringProof, stringPublic);
+    if (*proof_size < minProofSize || *public_size < minPublicSize) {
+        unsigned long long origProofSize = *proof_size;
+        unsigned long long origPublicSize = *public_size;
+        *proof_size = minProofSize;
+        *public_size = minPublicSize;
 
-        CheckAndUpdateBufferSizes(stringProof.length(), proof_size,
-                                  stringPublic.length(), public_size,
-                                  "Required");
+        CopyErrorFmt(error_msg, error_msg_maxsize,
+            "Buffer too small. Minimum required - proof: %llu (provided: %llu), public: %llu (provided: %llu)",
+            minProofSize, origProofSize, minPublicSize, origPublicSize);
+        return PROVER_ERROR_SHORT_BUFFER;
+    }
 
-        *proof_size = stringProof.length();
-        *public_size = stringPublic.length();
+    std::string stringProof;
+    std::string stringPublic;
 
-        std::strncpy(proof_buffer, stringProof.c_str(), *proof_size);
-        std::strncpy(public_buffer, stringPublic.c_str(), *public_size);
+    try {
+        prover->prove(wtns_buffer, wtns_size, stringProof, stringPublic);
 
     } catch(InvalidWitnessLengthException& e) {
         CopyError(error_msg, error_msg_maxsize, e);
         return PROVER_INVALID_WITNESS_LENGTH;
 
-    } catch(ShortBufferException& e) {
-        CopyError(error_msg, error_msg_maxsize, e);
-        return PROVER_ERROR_SHORT_BUFFER;
-
     } catch (std::exception& e) {
         CopyError(error_msg, error_msg_maxsize, e);
         return PROVER_ERROR;
@@ -388,10 +368,39 @@ groth16_prover_prove(
         return PROVER_ERROR;
 
     } catch (...) {
-        CopyError(error_msg, error_msg_maxsize, "unknown error");
+        CopyErrorFmt(error_msg, error_msg_maxsize, "unknown error");
+        return PROVER_ERROR;
+    }
+
+    // Check for overflow before adding 1 for null terminator
+    if (stringProof.length() >= ULLONG_MAX || stringPublic.length() >= ULLONG_MAX) {
+        CopyErrorFmt(error_msg, error_msg_maxsize, "Proof or public data too large");
         return PROVER_ERROR;
     }
 
+    unsigned long long requiredProofSize = stringProof.length() + 1;
+    unsigned long long requiredPublicSize = stringPublic.length() + 1;
+
+    if (*proof_size < requiredProofSize || *public_size < requiredPublicSize) {
+        unsigned long long origProofSize = *proof_size;
+        unsigned long long origPublicSize = *public_size;
+        *proof_size = requiredProofSize;
+        *public_size = requiredPublicSize;
+
+        CopyErrorFmt(error_msg, error_msg_maxsize,
+            "Buffer insufficient for generated proof. Required - proof: %llu (provided: %llu), public: %llu (provided: %llu)",
+            requiredProofSize, origProofSize, requiredPublicSize, origPublicSize);
+        return PROVER_ERROR_INSUFFICIENT_BUFFER;
+    }
+
+    std::memcpy(proof_buffer, stringProof.c_str(), stringProof.length());
+    proof_buffer[stringProof.length()] = '\0';
+    *proof_size = stringProof.length();
+
+    std::memcpy(public_buffer, stringPublic.c_str(), stringPublic.length());
+    public_buffer[stringPublic.length()] = '\0';
+    *public_size = stringPublic.length();
+
     return PROVER_OK;
 }
 

src/prover.h

@@ -5,11 +5,16 @@
 extern "C" {
 #endif
 
-//Error codes returned by the functions.
-#define PROVER_OK                     0x0
-#define PROVER_ERROR                  0x1
-#define PROVER_ERROR_SHORT_BUFFER     0x2
-#define PROVER_INVALID_WITNESS_LENGTH 0x3
+// Error codes returned by the functions.
+#define PROVER_OK                        0x0
+#define PROVER_ERROR                     0x1
+// Buffer smaller than minimum required size (checked before proof generation).
+// Proof generation is not attempted. Updated sizes are written to *_size params.
+#define PROVER_ERROR_SHORT_BUFFER        0x2
+#define PROVER_INVALID_WITNESS_LENGTH    0x3
+// Buffer smaller than actual proof output size (checked after proof generation).
+// Proof was generated but couldn't be written. Updated sizes are written to *_size params.
+#define PROVER_ERROR_INSUFFICIENT_BUFFER 0x4
 
 /**
  * Calculates buffer size to output public signals as json string
@@ -75,10 +80,27 @@ groth16_prover_create_zkey_file(
 
 /**
  * Proves 'wtns_buffer' and saves results to 'proof_buffer' and 'public_buffer'.
+ *
+ * @param prover_object Prover object created by groth16_prover_create
+ * @param wtns_buffer Witness data buffer
+ * @param wtns_size Size of witness buffer
+ * @param proof_buffer Buffer for proof output (JSON string)
+ * @param proof_size [in/out] On input: buffer size. On output: bytes written (excluding null terminator)
+ * @param public_buffer Buffer for public signals output (JSON string)
+ * @param public_size [in/out] On input: buffer size. On output: bytes written (excluding null terminator)
+ * @param error_msg Buffer for error message
+ * @param error_msg_maxsize Size of error message buffer
+ *
  * @return error code:
- *         PROVER_OK - in case of success
- *         PPOVER_ERROR - in case of an error
- *         PROVER_ERROR_SHORT_BUFFER - in case of a short buffer error, also updates proof_size and public_size with actual proof and public sizes
+ *         PROVER_OK - success, proof_size and public_size contain bytes written (excluding null terminator)
+ *         PROVER_ERROR_SHORT_BUFFER - buffers too small before proof generation,
+ *                                      both proof_size and public_size are always updated with minimum
+ *                                      required sizes (even if only one buffer is insufficient)
+ *         PROVER_ERROR_INSUFFICIENT_BUFFER - buffers too small for generated proof,
+ *                                             both proof_size and public_size are always updated with
+ *                                             required sizes (even if only one buffer is insufficient)
+ *         PROVER_INVALID_WITNESS_LENGTH - witness length doesn't match circuit
+ *         PROVER_ERROR - other error, see error_msg
  */
 int
 groth16_prover_prove(