[master] Merge branch 'trac2764'

src/bin/cfgmgr/plugins/tests/tsig_keys_test.py

@@ -86,7 +86,7 @@ class TSigKeysTest(unittest.TestCase):
         self.assertEqual("TSIG: Invalid TSIG key string: invalid.key",
                          tsig_keys.check({'keys': ['invalid.key']}))
         self.assertEqual(
-            "TSIG: Unexpected end of input in BASE decoder",
+            "TSIG: Incomplete input for base64: 123",
             tsig_keys.check({'keys': ['invalid.key:123']}))
 
     def test_bad_format(self):

src/lib/util/encode/base_n.cc

@@ -12,17 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <stdint.h>
-#include <cassert>
-#include <iterator>
-#include <string>
-#include <vector>
-
-#include <boost/archive/iterators/base64_from_binary.hpp>
-#include <boost/archive/iterators/binary_from_base64.hpp>
-#include <boost/archive/iterators/transform_width.hpp>
-#include <boost/math/common_factor.hpp>
-
 #include <util/encode/base32hex_from_binary.h>
 #include <util/encode/binary_from_base32hex.h>
 #include <util/encode/base16_from_binary.h>
@@ -32,6 +21,18 @@
 
 #include <exceptions/exceptions.h>
 
+#include <boost/archive/iterators/base64_from_binary.hpp>
+#include <boost/archive/iterators/binary_from_base64.hpp>
+#include <boost/archive/iterators/transform_width.hpp>
+#include <boost/math/common_factor.hpp>
+
+#include <stdint.h>
+#include <stdexcept>
+#include <cassert>
+#include <iterator>
+#include <string>
+#include <vector>
+
 using namespace std;
 using namespace boost::archive::iterators;
 
@@ -110,15 +111,15 @@ public:
                      const vector<uint8_t>::const_iterator& base_end) :
         base_(base), base_end_(base_end), in_pad_(false)
     {}
-    EncodeNormalizer& operator++() {
-        if (!in_pad_) {
-            ++base_;
-        }
-        if (base_ == base_end_) {
-            in_pad_ = true;
-        }
+    EncodeNormalizer& operator++() { // prefix version
+        increment();
         return (*this);
     }
+    EncodeNormalizer operator++(int) { // postfix version
+        const EncodeNormalizer copy = *this;
+        increment();
+        return (copy);
+    }
     const uint8_t& operator*() const {
         if (in_pad_) {
             return (BINARY_ZERO_CODE);
@@ -130,11 +131,24 @@ public:
         return (base_ == other.base_);
     }
 private:
+    void increment() {
+        if (!in_pad_) {
+            ++base_;
+        }
+        if (base_ == base_end_) {
+            in_pad_ = true;
+        }
+    }
     vector<uint8_t>::const_iterator base_;
     const vector<uint8_t>::const_iterator base_end_;
     bool in_pad_;
 };
 
+// An internally caught exception to unify a few possible cases of the same
+// error.
+class IncompleteBaseInput : public std::exception {
+};
+
 // DecodeNormalizer is an input iterator intended to be used as a filter
 // between the encoded baseX stream and binary_from_baseXX.
 // A DecodeNormalizer object is configured with three string iterators
@@ -156,16 +170,20 @@ public:
     DecodeNormalizer(const char base_zero_code,
                      const string::const_iterator& base,
                      const string::const_iterator& base_beginpad,
-                     const string::const_iterator& base_end) :
+                     const string::const_iterator& base_end,
+                     size_t* char_count) :
         base_zero_code_(base_zero_code),
         base_(base), base_beginpad_(base_beginpad), base_end_(base_end),
-        in_pad_(false)
+        in_pad_(false), char_count_(char_count)
     {
         // Skip beginning spaces, if any.  We need do it here because
         // otherwise the first call to operator*() would be confused.
         skipSpaces();
     }
     DecodeNormalizer& operator++() {
+        if (base_ < base_end_) {
+            ++*char_count_;
+        }
         ++base_;
         skipSpaces();
         if (base_ == base_beginpad_) {
@@ -188,15 +206,29 @@ public:
     }
     const char& operator*() const {
         if (base_ == base_end_) {
-            // binary_from_baseX calls this operator when it needs more bits
+            // binary_from_baseX can call this operator when it needs more bits
             // even if the internal iterator (base_) has reached its end
             // (if that happens it means the input is an incomplete baseX
             // string and should be rejected).  So this is the only point
             // we can catch and reject this type of invalid input.
-            isc_throw(BadValue, "Unexpected end of input in BASE decoder");
+            //
+            // More recent versions of Boost fixed the behavior and the
+            // out-of-range call to this operator doesn't happen.  It's good,
+            // but in that case we need to catch incomplete baseX input in
+            // a different way.  It's done via char_count_ and after the
+            // completion of decoding.
+            throw IncompleteBaseInput(); // throw this now and convert it
         }
-        if (in_pad_) {
-            return (base_zero_code_);
+        if (*base_ == BASE_PADDING_CHAR) {
+            // Padding can only happen at the end of the input string.  We can
+            // detect any violation of this by checking in_pad_, which is
+            // true iff we are on or after the first valid sequence of padding
+            // characters.
+            if (in_pad_) {
+                return (base_zero_code_);
+            } else {
+                isc_throw(BadValue, "Intermediate padding found");
+            }
         } else {
             return (*base_);
         }
@@ -210,6 +242,9 @@ private:
     const string::const_iterator base_beginpad_;
     const string::const_iterator base_end_;
     bool in_pad_;
+    // Store number of non-space decoded characters (incl. pad) here.  Define
+    // it as a pointer so we can carry it over to any copied objects.
+    size_t* char_count_;
 };
 
 // BitsPerChunk: number of bits to be converted using the baseN mapping table.
@@ -331,10 +366,24 @@ BaseNTransformer<BitsPerChunk, BaseZeroCode, Encoder, Decoder>::decode(
     const size_t padbytes = padbits / 8;
 
     try {
+        size_t char_count = 0;
         result.assign(Decoder(DecodeNormalizer(BaseZeroCode, input.begin(),
-                                               srit.base(), input.end())),
+                                               srit.base(), input.end(),
+                                               &char_count)),
                       Decoder(DecodeNormalizer(BaseZeroCode, input.end(),
-                                               input.end(), input.end())));
+                                               input.end(), input.end(),
+                                               NULL)));
+
+        // Number of bits of the conversion result including padding must be
+        // a multiple of 8; otherwise the decoder reaches the end of input
+        // with some incomplete bits of data, which is invalid.
+        if (((char_count * BitsPerChunk) % 8) != 0) {
+            throw IncompleteBaseInput(); // catch this immediately below
+        }
+    } catch (const IncompleteBaseInput&) {
+        // we unify error handling for incomplete input here.
+        isc_throw(BadValue, "Incomplete input for " << algorithm
+                  << ": " << input);
     } catch (const dataflow_exception& ex) {
         // convert any boost exceptions into our local one.
         isc_throw(BadValue, ex.what());