From 703bb3452a84fe36dcaa554535d6ce6e9802a66a Mon Sep 17 00:00:00 2001
From: Mike Hamburg <mike@shiftleft.org>
Date: Wed, 13 Jul 2022 14:43:37 +0200
Subject: [PATCH] Fix two security bugs. Point::steg_encode was leaving the 24
 high bits of the buffer as zero. It also ignored the size parameter.  The
 size parameter has now been removed, the zeros fixed and a test added to make
 sure that it is fixed.

Per https://github.com/MystenLabs/ed25519-unsafe-libs, deprecate eddsa signing
with separate pubkey and privkey input.  Instead decaf_ed*_keypair_sign
---
 src/generator/template.py    |  2 +-
 src/per_curve/eddsa.tmpl.h   |  4 ++--
 src/per_curve/point.tmpl.hxx |  5 ++---
 test/test_decaf.cxx          | 12 +++++++++---
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/generator/template.py b/src/generator/template.py
index e7ba2fd..31ed547 100644
--- a/src/generator/template.py
+++ b/src/generator/template.py
@@ -43,7 +43,7 @@ def fillin(template,data):
     ret = ""
     while True:
         dollars = template.find("$(",position)
-        if dollars is -1: return ret + template[position:]
+        if dollars == -1: return ret + template[position:]
         ret += template[position:dollars]
         position = dollars + 2
         parens = 1
diff --git a/src/per_curve/eddsa.tmpl.h b/src/per_curve/eddsa.tmpl.h
index 69b5303..d3d253e 100644
--- a/src/per_curve/eddsa.tmpl.h
+++ b/src/per_curve/eddsa.tmpl.h
@@ -45,8 +45,8 @@ $("DECAF_API_VIS extern const uint8_t * const DECAF_ED" + gf_shortname + "_NO_CO
 #define $(C_NS)_EDDSA_DECODE_RATIO ($(cofactor) / $(eddsa_encode_ratio))
     
 #ifndef DECAF_EDDSA_NON_KEYPAIR_API_IS_DEPRECATED
-/** If 1, add deprecation attribute to non-keypair API functions. For now, deprecate in Doxygen only. */
-#define DECAF_EDDSA_NON_KEYPAIR_API_IS_DEPRECATED 0
+/** If 1, add deprecation attribute to non-keypair API functions. Now deprecated. */
+#define DECAF_EDDSA_NON_KEYPAIR_API_IS_DEPRECATED 1
 #endif
 
 /** @cond internal */
diff --git a/src/per_curve/point.tmpl.hxx b/src/per_curve/point.tmpl.hxx
index bed64aa..119fd18 100644
--- a/src/per_curve/point.tmpl.hxx
+++ b/src/per_curve/point.tmpl.hxx
@@ -551,12 +551,11 @@ public:
     }
 
     /** Steganographically encode this */
-    inline SecureBuffer steg_encode(Rng &rng, size_t size=STEG_BYTES) const /*throw(std::bad_alloc, LengthException)*/ {
-        if (size <= HASH_BYTES + 4 || size > 2*HASH_BYTES) throw LengthException();
+    inline SecureBuffer steg_encode(Rng &rng) const /*throw(std::bad_alloc, LengthException)*/ {
         SecureBuffer out(STEG_BYTES);
         decaf_error_t done;
         do {
-            rng.read(Buffer(out).slice(HASH_BYTES-4,STEG_BYTES-HASH_BYTES+1));
+            rng.read(Buffer(out).slice(HASH_BYTES-4,STEG_BYTES-HASH_BYTES+4));
             uint32_t hint = 0;
             for (int i=0; i<4; i++) { hint |= uint32_t(out[HASH_BYTES-4+i])<<(8*i); }
             done = invert_elligator(out, hint);
diff --git a/test/test_decaf.cxx b/test/test_decaf.cxx
index 40f8f80..841a40e 100644
--- a/test/test_decaf.cxx
+++ b/test/test_decaf.cxx
@@ -72,8 +72,8 @@ static void print(const char *name, const Scalar &x) {
 
 static void hexprint(const char *name, const SecureBuffer &buffer) {
     printf("  %s = 0x", name);
-    for (auto i = buffer.rbegin(); i!= buffer.rend(); ++i) {
-        printf("%02x", *i);
+    for (int i=buffer.size()-1; i>=0; i--) {
+        printf("%02x", buffer[i]);
     }
     printf("\n");
 }
@@ -292,7 +292,13 @@ static void test_elligator() {
         }
         
         Point t(rng);
-        point_check(test,t,t,t,0,0,t,Point::from_hash(t.steg_encode(rng)),"steg round-trip");
+        SecureBuffer bsteg = t.steg_encode(rng);
+        if (bsteg[Point::STEG_BYTES-1] == 0 && bsteg[Point::STEG_BYTES-2] == 0 && bsteg[Point::STEG_BYTES-3] == 0) {
+            test.fail();
+            printf("    Steg is nonuniform (probability of false failure = 2^-24)\n");
+            hexprint("    steg buffer:", bsteg);
+        }
+        point_check(test,t,t,t,0,0,t,Point::from_hash(bsteg),"steg round-trip");
         
         FixedArrayBuffer<Point::HASH_BYTES> b3(rng), b4(b3);
         t = Point::from_hash(b3);