Repository: randombit/botan
Pull request #2761 (2021-06-22 21:34:41)

Tweak how BigInt string formatting works
to_hex_string and to_dec_string ignored the sign, as a result this also caused asn1printer to print negative numbers wrong.

to_hex_string includes 0x prefix

And if the hex encoded has a leading 0 nibble that is printed instead of being ignored now.


Patch:
diff --git a/src/fuzzer/asn1.cpp b/src/fuzzer/asn1.cpp
index c3fffb15c..13a2517cb 100644
--- a/src/fuzzer/asn1.cpp
+++ b/src/fuzzer/asn1.cpp
@@ -25,6 +25,11 @@ class ASN1_Parser final : public Botan::ASN1_Formatter
          {
          return "";
          }
+
+      std::string format_bn(const Botan::BigInt&) const override
+         {
+         return "";
+         }
    };
 
 void fuzz(const uint8_t in[], size_t len)
diff --git a/src/lib/asn1/asn1_print.cpp b/src/lib/asn1/asn1_print.cpp
index 34c671483..1a1654fef 100644
--- a/src/lib/asn1/asn1_print.cpp
+++ b/src/lib/asn1/asn1_print.cpp
@@ -172,11 +172,7 @@ void ASN1_Formatter::decode(std::ostream& output,
             data.decode(number, ASN1_Type::Enumerated, class_tag);
             }
 
-         std::vector<uint8_t> rep = BigInt::encode(number);
-         if(rep.empty()) // if zero
-            rep.resize(1);
-
-         output << format(type_tag, class_tag, level, length, hex_encode(rep));
+         output << format(type_tag, class_tag, level, length, format_bn(number));
          }
       else if(type_tag == ASN1_Type::Boolean)
          {
@@ -324,4 +320,12 @@ std::string ASN1_Pretty_Printer::format_bin(ASN1_Type /*type_tag*/,
       return hex_encode(vec);
    }
 
+std::string ASN1_Pretty_Printer::format_bn(const BigInt& bn) const
+   {
+   if(bn.bits() < 16)
+      return bn.to_dec_string();
+   else
+      return bn.to_hex_string();
+   }
+
 }
diff --git a/src/lib/asn1/asn1_print.h b/src/lib/asn1/asn1_print.h
index 53ed42de6..94dc48d6a 100644
--- a/src/lib/asn1/asn1_print.h
+++ b/src/lib/asn1/asn1_print.h
@@ -14,6 +14,7 @@
 
 namespace Botan {
 
+class BigInt;
 class BER_Decoder;
 
 /**
@@ -58,13 +59,18 @@ class BOTAN_PUBLIC_API(2,4) ASN1_Formatter
 
       /**
       * This is called to format binary elements that we don't know how to
-      * convert to a string The result will be passed as value to format; the
+      * convert to a string. The result will be passed as value to format; the
       * tags are included as a hint to aid decoding.
       */
       virtual std::string format_bin(ASN1_Type type_tag,
                                      ASN1_Class class_tag,
                                      const std::vector<uint8_t>& vec) const = 0;
 
+      /**
+      * This is called to format integers
+      */
+      virtual std::string format_bn(const BigInt& bn) const = 0;
+
    private:
       void decode(std::ostream& output,
                   BER_Decoder& decoder,
@@ -114,6 +120,8 @@ class BOTAN_PUBLIC_API(2,4) ASN1_Pretty_Printer final : public ASN1_Formatter
                              ASN1_Class class_tag,
                              const std::vector<uint8_t>& vec) const override;
 
+      std::string format_bn(const BigInt& bn) const override;
+
       const size_t m_print_limit;
       const size_t m_print_binary_limit;
       const size_t m_initial_level;
diff --git a/src/lib/math/bigint/big_code.cpp b/src/lib/math/bigint/big_code.cpp
index 4b23bd5a5..a4bd03a04 100644
--- a/src/lib/math/bigint/big_code.cpp
+++ b/src/lib/math/bigint/big_code.cpp
@@ -17,7 +17,10 @@ std::string BigInt::to_dec_string() const
    copy.set_sign(Positive);
 
    uint8_t remainder;
-   std::vector<uint8_t> digits;
+   secure_vector<uint8_t> digits;
+
+   // (over-)estimate of the number of digits, log2(10) ~= 3.32
+   digits.reserve(static_cast<size_t>(3.4 * this->bits()));
 
    while(copy > 0)
       {
@@ -33,6 +36,9 @@ std::string BigInt::to_dec_string() const
 
    std::string s;
 
+   if(is_negative())
+      s += "-";
+
    for(auto i = digits.rbegin(); i != digits.rend(); ++i)
       {
       s.push_back(*i + '0');
@@ -46,11 +52,17 @@ std::string BigInt::to_dec_string() const
 
 std::string BigInt::to_hex_string() const
    {
-   const std::vector<uint8_t> bits = BigInt::encode(*this);
+   std::vector<uint8_t> bits = BigInt::encode(*this);
+
    if(bits.empty())
-      return "00";
-   else
-      return hex_encode(bits);
+      bits.push_back(0);
+
+   std::string hrep;
+   if(is_negative())
+      hrep += "-";
+   hrep += "0x";
+   hrep += hex_encode(bits);
+   return hrep;
    }
 
 /*
@@ -80,8 +92,6 @@ void BigInt::encode_1363(uint8_t output[], size_t bytes, const BigInt& n)
 */
 secure_vector<uint8_t> BigInt::encode_fixed_length_int_pair(const BigInt& n1, const BigInt& n2, size_t bytes)
    {
-   if(n1.is_negative() || n2.is_negative())
-      throw Encoding_Error("encode_fixed_length_int_pair: values must be positive");
    if(n1.bytes() > bytes || n2.bytes() > bytes)
       throw Encoding_Error("encode_fixed_length_int_pair: values too large to encode properly");
    secure_vector<uint8_t> output(2 * bytes);
diff --git a/src/lib/math/bigint/big_io.cpp b/src/lib/math/bigint/big_io.cpp
index b31315eac..ae2855fcc 100644
--- a/src/lib/math/bigint/big_io.cpp
+++ b/src/lib/math/bigint/big_io.cpp
@@ -16,17 +16,17 @@ namespace Botan {
 */
 std::ostream& operator<<(std::ostream& stream, const BigInt& n)
    {
-   size_t base = 10;
-   if(stream.flags() & std::ios::hex)
-      base = 16;
-   if(stream.flags() & std::ios::oct)
+   const auto stream_flags = stream.flags();
+   if(stream_flags & std::ios::oct)
       throw Invalid_Argument("Octal output of BigInt not supported");
 
-   if(n == 0)
+   const size_t base = (stream_flags & std::ios::hex) ? 16 : 10;
+
+   if(n.is_zero())
       stream.write("0", 1);
    else
       {
-      if(n < 0)
+      if(n.is_negative())
          stream.write("-", 1);
 
       std::string enc;
@@ -36,10 +36,7 @@ std::ostream& operator<<(std::ostream& stream, const BigInt& n)
       else
          enc = n.to_hex_string();
 
-      size_t skip = 0;
-      while(skip < enc.size() && enc[skip] == '0')
-         ++skip;
-      stream.write(&enc[skip], enc.size() - skip);
+      stream.write(enc.data(), enc.size());
       }
    if(!stream.good())
       throw Stream_IO_Error("BigInt output operator has failed");
diff --git a/src/python/botan2.py b/src/python/botan2.py
index cd6f98b7c..6e06e5747 100755
--- a/src/python/botan2.py
+++ b/src/python/botan2.py
@@ -1498,13 +1498,9 @@ class MPI(object): # pylint: disable=too-many-public-methods
         return self.__obj
 
     def __int__(self):
-        out = create_string_buffer(2*self.byte_count() + 1)
+        out = create_string_buffer(2*self.byte_count() + 3)
         _DLL.botan_mp_to_hex(self.__obj, out)
-        val = int(out.value, 16)
-        if self.is_negative():
-            return -val
-        else:
-            return val
+        return int(out.value, 16)
 
     def __repr__(self):
         # Should have a better size estimate than this ...


Test patch:
diff --git a/src/scripts/test_cli.py b/src/scripts/test_cli.py
index 2fe2ade16..1679e0ade 100755
--- a/src/scripts/test_cli.py
+++ b/src/scripts/test_cli.py
@@ -707,11 +707,11 @@ def cli_dl_group_info_tests(_tmp_dir):
 def cli_ec_group_info_tests(_tmp_dir):
 
     # pylint: disable=line-too-long
-    secp256r1_info = """P = FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF
-A = FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC
-B = 5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B
-N = FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551
-G = 6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296,4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5"""
+    secp256r1_info = """P = 0xFFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF
+A = 0xFFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC
+B = 0x5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B
+N = 0xFFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551
+G = 0x6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296,0x4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5"""
 
     secp256r1_pem = """-----BEGIN EC PARAMETERS-----
 MIHgAgEBMCwGByqGSM49AQECIQD/////AAAAAQAAAAAAAAAAAAAAAP//////////
@@ -807,11 +807,11 @@ MCACAQUTBnN0cmluZzEGAQH/AgFjBAUAAAAAAAMEAP///w==
 """
 
     expected = """d= 0, l=  32: SEQUENCE
-  d= 1, l=   1:  INTEGER                                    05
+  d= 1, l=   1:  INTEGER                                    5
   d= 1, l=   6:  PRINTABLE STRING                           string
   d= 1, l=   6:  SET
   d= 2, l=   1:   BOOLEAN                                   true
-  d= 2, l=   1:   INTEGER                                   63
+  d= 2, l=   1:   INTEGER                                   99
   d= 1, l=   5:  OCTET STRING                               0000000000
   d= 1, l=   4:  BIT STRING                                 FFFFFF"""
 
diff --git a/src/tests/data/asn1_print/output1.txt b/src/tests/data/asn1_print/output1.txt
index 68f7389ea..b37da505b 100644
--- a/src/tests/data/asn1_print/output1.txt
+++ b/src/tests/data/asn1_print/output1.txt
@@ -1,8 +1,8 @@
   d= 0, l=  32: SEQUENCE
-  d= 1, l=   1:  INTEGER                                    05
+  d= 1, l=   1:  INTEGER                                    5
   d= 1, l=   6:  PRINTABLE STRING                           string
   d= 1, l=   6:  SET
   d= 2, l=   1:   BOOLEAN                                   true
-  d= 2, l=   1:   INTEGER                                   63
+  d= 2, l=   1:   INTEGER                                   99
   d= 1, l=   5:  OCTET STRING                               0000000000
   d= 1, l=   4:  BIT STRING                                 FFFFFF
diff --git a/src/tests/data/asn1_print/output2.txt b/src/tests/data/asn1_print/output2.txt
index 523f292eb..8f9bd8b40 100644
--- a/src/tests/data/asn1_print/output2.txt
+++ b/src/tests/data/asn1_print/output2.txt
@@ -1,10 +1,10 @@
   d= 0, l=  38: SEQUENCE
-  d= 1, l=   1:  INTEGER                                    00
+  d= 1, l=   1:  INTEGER                                    0
   d= 1, l=   6:  OCTET STRING                               public
   d= 1, l=  25:  cons [0] context
-  d= 2, l=   1:   INTEGER                                   26
-  d= 2, l=   1:   INTEGER                                   00
-  d= 2, l=   1:   INTEGER                                   00
+  d= 2, l=   1:   INTEGER                                   38
+  d= 2, l=   1:   INTEGER                                   0
+  d= 2, l=   1:   INTEGER                                   0
   d= 2, l=  14:   SEQUENCE
   d= 3, l=  12:    SEQUENCE
   d= 4, l=   8:     OBJECT                                  1.3.6.1.2.1.1.2.0
diff --git a/src/tests/data/asn1_print/output5.txt b/src/tests/data/asn1_print/output5.txt
index 4769b98ee..a6ec8d93c 100644
--- a/src/tests/data/asn1_print/output5.txt
+++ b/src/tests/data/asn1_print/output5.txt
@@ -1,8 +1,8 @@
   d= 0, l=1779: SEQUENCE
   d= 1, l=1499:  SEQUENCE
   d= 2, l=   3:   cons [0] context
-  d= 3, l=   1:    INTEGER                                  02
-  d= 2, l=  16:   INTEGER                                   03EB7E1D9B4C4C7B145FF6A902CE81F9
+  d= 3, l=   1:    INTEGER                                  2
+  d= 2, l=  16:   INTEGER                                   0x03EB7E1D9B4C4C7B145FF6A902CE81F9
   d= 2, l=  13:   SEQUENCE
   d= 3, l=   9:    OBJECT                                   RSA/EMSA3(SHA-256) [1.2.840.113549.1.1.11]
   d= 3, l=   0:    NULL
@@ -57,8 +57,8 @@
   d= 4, l=   0:     NULL
   d= 3, l= 271:    BIT STRING
   d= 4, l= 266:     SEQUENCE
-  d= 5, l= 257:      INTEGER                                B5AA73E7BDFFA06C6A1D0F0E116386ACB98C55C27EEC1FDF0164539D337F4D66EE0824CE6355927411770262AC284619AE06F2BAADF3D9E03E40850F42846F821AF49219E7EF8434AEF76DB5E3E9F17AFB5CE04E45959B77CC9B55819EC501B5979F345DF10051AC456926A48CD44F0EC8FBD3E8E91D5A76C35C882DC8FA95275AC2150ACF990D4019B96E55497CB1F46A842643C64168DFD8A31AC4A1E808DF710C53363ADAF82B5F62C9A2AAEE1FEC64880D951E7D48BDB3FDF724E25F67F373D2EC1426B7E4BA2B60442A42320D3FEF96640B6E795CF8C4FEB28E1608CB968BBB83B65F96BB5175682095310CE5FD02805AA9273326F7A9E8B33730EB8EDD
-  d= 5, l=   3:      INTEGER                                010001
+  d= 5, l= 257:      INTEGER                                0xB5AA73E7BDFFA06C6A1D0F0E116386ACB98C55C27EEC1FDF0164539D337F4D66EE0824CE6355927411770262AC284619AE06F2BAADF3D9E03E40850F42846F821AF49219E7EF8434AEF76DB5E3E9F17AFB5CE04E45959B77CC9B55819EC501B5979F345DF10051AC456926A48CD44F0EC8FBD3E8E91D5A76C35C882DC8FA95275AC2150ACF990D4019B96E55497CB1F46A842643C64168DFD8A31AC4A1E808DF710C53363ADAF82B5F62C9A2AAEE1FEC64880D951E7D48BDB3FDF724E25F67F373D2EC1426B7E4BA2B60442A42320D3FEF96640B6E795CF8C4FEB28E1608CB968BBB83B65F96BB5175682095310CE5FD02805AA9273326F7A9E8B33730EB8EDD
+  d= 5, l=   3:      INTEGER                                0x010001
   d= 2, l= 872:   cons [3] context
   d= 3, l= 868:    SEQUENCE
   d= 4, l= 153:     SEQUENCE
diff --git a/src/tests/data/asn1_print/output6.txt b/src/tests/data/asn1_print/output6.txt
index db97ab06e..bba37fb16 100644
--- a/src/tests/data/asn1_print/output6.txt
+++ b/src/tests/data/asn1_print/output6.txt
@@ -1,8 +1,8 @@
   d= 0, l=2049: SEQUENCE
   d= 1, l=1513:  SEQUENCE
   d= 2, l=   3:   cons [0] context
-  d= 3, l=   1:    INTEGER                                  02
-  d= 2, l=   9:   INTEGER                                   C1E986160DA8E982
+  d= 3, l=   1:    INTEGER                                  2
+  d= 2, l=   9:   INTEGER                                   0xC1E986160DA8E982
   d= 2, l=  13:   SEQUENCE
   d= 3, l=   9:    OBJECT                                   RSA/EMSA3(SHA-512) [1.2.840.113549.1.1.13]
   d= 3, l=   0:    NULL
@@ -77,8 +77,8 @@
   d= 4, l=   0:     NULL
   d= 3, l= 527:    BIT STRING
   d= 4, l= 522:     SEQUENCE
-  d= 5, l= 513:      INTEGER                                B591048C4E486F34E9DC08627FC2375162236984B82CB130BEFF517CFC38F84BCE5C65A874DAB2621AE0BCE7E33563E0EDE934FD5F8823159F07848808227460C1ED88261706F4281334359DFBB81BD1353FC179610AF1A8C8C865DC00EA23B3A89BE6BD03BA85A9EC827D60565905E22D6A584ED1380AE150280CEE397E98A012F380464007862443BC077CB95F421AF31712D9683CDB6DFFBAF3C8BA5BA566AE523D459D6177346D4D840E27886B7C01C5B890D78A2E27BBA8DD2F9A2812E157D62F921C65962548069DCDB7D06DE181DE0E9570D66F87220CE28B628AB55906F3EE0C210F7051E8F4858AF8B9A92D09E46AF2D9CBA5BFCFAD168CDF604491A4B06603B114CAF7031F065E7EEEFA53C575F3490C059D2E32DDC76AC4D4C4C710683B97FD1BE591BC61055186D88F9A0391B307B6F91ED954DAA36F9ACD6A1E14AA2E4ADF17464B54DB18DBB6FFE30080246547370436CE4E77BAE5DE6FE0F3F9D6E7FFBEB461E794E92FB0951F8AAE61A412CCE9B21074635C8BE327AE1A0F6B4A646EB0F8463BC63BF845530435D19E802511EC9F66C3496952D8BECB69B0AA4D4C41F60515FE7DCBB89319CDDA59BA6AEA4BE3CEAE718E6FCB6CCD7DB9FC50BB15B12F3665B0AA307289C2E6DD4B111CE48BA2D9EFDB5A6B9A506069334FB34F6FC7AE330F0B34208AAC80DF3266FDD90465876BA2CB898D9505315B6E7B
-  d= 5, l=   3:      INTEGER                                010001
+  d= 5, l= 513:      INTEGER                                0xB591048C4E486F34E9DC08627FC2375162236984B82CB130BEFF517CFC38F84BCE5C65A874DAB2621AE0BCE7E33563E0EDE934FD5F8823159F07848808227460C1ED88261706F4281334359DFBB81BD1353FC179610AF1A8C8C865DC00EA23B3A89BE6BD03BA85A9EC827D60565905E22D6A584ED1380AE150280CEE397E98A012F380464007862443BC077CB95F421AF31712D9683CDB6DFFBAF3C8BA5BA566AE523D459D6177346D4D840E27886B7C01C5B890D78A2E27BBA8DD2F9A2812E157D62F921C65962548069DCDB7D06DE181DE0E9570D66F87220CE28B628AB55906F3EE0C210F7051E8F4858AF8B9A92D09E46AF2D9CBA5BFCFAD168CDF604491A4B06603B114CAF7031F065E7EEEFA53C575F3490C059D2E32DDC76AC4D4C4C710683B97FD1BE591BC61055186D88F9A0391B307B6F91ED954DAA36F9ACD6A1E14AA2E4ADF17464B54DB18DBB6FFE30080246547370436CE4E77BAE5DE6FE0F3F9D6E7FFBEB461E794E92FB0951F8AAE61A412CCE9B21074635C8BE327AE1A0F6B4A646EB0F8463BC63BF845530435D19E802511EC9F66C3496952D8BECB69B0AA4D4C41F60515FE7DCBB89319CDDA59BA6AEA4BE3CEAE718E6FCB6CCD7DB9FC50BB15B12F3665B0AA307289C2E6DD4B111CE48BA2D9EFDB5A6B9A506069334FB34F6FC7AE330F0B34208AAC80DF3266FDD90465876BA2CB898D9505315B6E7B
+  d= 5, l=   3:      INTEGER                                0x010001
   d= 2, l= 475:   cons [3] context
   d= 3, l= 471:    SEQUENCE
   d= 4, l=   9:     SEQUENCE
diff --git a/src/tests/test_bigint.cpp b/src/tests/test_bigint.cpp
index dbf642c9c..459263b47 100644
--- a/src/tests/test_bigint.cpp
+++ b/src/tests/test_bigint.cpp
@@ -208,7 +208,7 @@ class BigInt_Unit_Tests final : public Test
 
          oss.str("");
          oss << std::hex << n;
-         result.test_eq("output 33 hex", oss.str(), "21");
+         result.test_eq("output 33 hex", oss.str(), "0x21");
 
          result.test_throws("octal output not supported", [&]() { oss << std::oct << n; });
 
diff --git a/src/tests/test_ffi.cpp b/src/tests/test_ffi.cpp
index b6ed2239d..143371cb5 100644
--- a/src/tests/test_ffi.cpp
+++ b/src/tests/test_ffi.cpp
@@ -1416,7 +1416,7 @@ class FFI_Unit_Tests final : public Test
          result.test_eq("botan_mp_num_bits", x_bits, 9);
 
          TEST_FFI_OK(botan_mp_to_hex, (x, str_buf));
-         result.test_eq("botan_mp_to_hex", std::string(str_buf), "0103");
+         result.test_eq("botan_mp_to_hex", std::string(str_buf), "0x0103");
 
          uint32_t x_32;
          TEST_FFI_OK(botan_mp_to_uint32, (x, &x_32));
@@ -1427,11 +1427,11 @@ class FFI_Unit_Tests final : public Test
          TEST_FFI_OK(botan_mp_set_bit, (x, 87));
          TEST_FFI_RC(1, botan_mp_get_bit, (x, 87));
          TEST_FFI_OK(botan_mp_to_hex, (x, str_buf));
-         result.test_eq("botan_mp_set_bit", std::string(str_buf), "8000000000000000000103");
+         result.test_eq("botan_mp_set_bit", std::string(str_buf), "0x8000000000000000000103");
 
          TEST_FFI_OK(botan_mp_clear_bit, (x, 87));
          TEST_FFI_OK(botan_mp_to_hex, (x, str_buf));
-         result.test_eq("botan_mp_set_bit", std::string(str_buf), "0103");
+         result.test_eq("botan_mp_set_bit", std::string(str_buf), "0x0103");
 
          botan_mp_t y;
          TEST_FFI_OK(botan_mp_init, (&y));
@@ -1467,7 +1467,7 @@ class FFI_Unit_Tests final : public Test
          TEST_FFI_OK(botan_mp_sub, (r, x, y));
          str_len = sizeof(str_buf);
          TEST_FFI_OK(botan_mp_to_str, (r, 10, str_buf, &str_len));
-         result.test_eq("botan_mp_sub", std::string(str_buf), "4943984178");
+         result.test_eq("botan_mp_sub", std::string(str_buf), "-4943984178");
          TEST_FFI_RC(1, botan_mp_is_negative, (r));
 
          TEST_FFI_OK(botan_mp_lshift, (r, x, 39));