Implement the recursion limit in the CBOR decoder API

It's very easy for a short packet to contain a very high number of
nested arrays (a packet with N bytes can be N-1 nested arrays), which
could cause stack overflow problems in devices using
cbor_value_map_find_value, so ensure that we won't crash.

The actual value needs to be chosen by the implementor depending on the
stack size of their devices and the expected usage.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>

TODO

@@ -23,4 +23,3 @@
 * (unlikely) Add API for checking the pairing of a tag and the tagged type
 * Write tests for error conditions
 * Fuzzy-test the decoder
-* Add recursion limit to recursive functions (advance, map_find)

src/cbor.h

@@ -117,6 +117,7 @@ typedef enum CborError {
 
     /* internal implementation errors */
     CborErrorDataTooLarge = 1024,
+    CborErrorNestingTooDeep,
     CborErrorInternalError = ~0U
 } CborError;
 

src/cborerrorstrings.c

@@ -85,6 +85,9 @@ const char *cbor_error_string(CborError error)
     case CborErrorDataTooLarge:
         return _("internal error: data too large");
 
+    case CborErrorNestingTooDeep:
+        return _("internal error: too many nested containers found in recursive function");
+
     case CborErrorInternalError:
         return _("internal error");
     }

src/cborparser.c

@@ -32,6 +32,10 @@
 #include <stdlib.h>
 #include <string.h>
 
+#ifndef CBOR_PARSER_MAX_RECURSIONS
+#  define CBOR_PARSER_MAX_RECURSIONS 1024
+#endif
+
 /**
  * \typedef CborValue
  * This type contains one value parsed from the CBOR stream.
@@ -307,6 +311,34 @@ CborError cbor_value_advance_fixed(CborValue *it)
     return advance_internal(it);
 }
 
+static CborError advance_recursive(CborValue *it, int nestingLevel)
+{
+    if (is_fixed_type(it->type))
+        return advance_internal(it);
+
+    if (!cbor_value_is_container(it)) {
+        size_t len = SIZE_MAX;
+        return cbor_value_copy_string(it, NULL, &len, it);
+    }
+
+    // map or array
+    if (nestingLevel == CBOR_PARSER_MAX_RECURSIONS)
+        return CborErrorNestingTooDeep;
+
+    CborError err;
+    CborValue recursed;
+    err = cbor_value_enter_container(it, &recursed);
+    if (err)
+        return err;
+    while (!cbor_value_at_end(&recursed)) {
+        err = advance_recursive(&recursed, nestingLevel + 1);
+        if (err)
+            return err;
+    }
+    return cbor_value_leave_container(it, &recursed);
+}
+
+
 /**
  * Advances the CBOR value \a it by one element, skipping over containers.
  * Unlike cbor_value_advance_fixed(), this function can be called on a CBOR
@@ -323,26 +355,7 @@ CborError cbor_value_advance(CborValue *it)
     assert(it->type != CborInvalidType);
     if (!it->remaining)
         return CborErrorAdvancePastEOF;
-    if (is_fixed_type(it->type))
-        return advance_internal(it);
-
-    if (!cbor_value_is_container(it)) {
-        size_t len = SIZE_MAX;
-        return cbor_value_copy_string(it, NULL, &len, it);
-    }
-
-    // map or array
-    CborError err;
-    CborValue recursed;
-    err = cbor_value_enter_container(it, &recursed);
-    if (err)
-        return err;
-    while (!cbor_value_at_end(&recursed)) {
-        err = cbor_value_advance(&recursed);
-        if (err)
-            return err;
-    }
-    return cbor_value_leave_container(it, &recursed);
+    return advance_recursive(it, 0);
 }
 
 /**
@@ -361,7 +374,6 @@ CborError cbor_value_skip_tag(CborValue *it)
     return CborNoError;
 }
 
-
 /**
  * \fn bool cbor_value_is_container(const CborValue *it)
  *

tests/parser/parser.pro

@@ -2,5 +2,6 @@ SOURCES += tst_parser.cpp
 
 CONFIG += testcase parallel_test c++11
 QT = core testlib
+DEFINES += CBOR_PARSER_MAX_RECURSIONS=16
 
 include(../../src/src.pri)

tests/parser/tst_parser.cpp

@@ -76,6 +76,8 @@ private slots:
     void resumeParsing();
     void endPointer_data();
     void endPointer();
+    void recursionLimit_data();
+    void recursionLimit();
 };
 
 char toHexUpper(unsigned n)
@@ -1316,5 +1318,75 @@ void tst_Parser::endPointer()
     QCOMPARE(int(first.ptr - data.constBegin()), offset);
 }
 
+void tst_Parser::recursionLimit_data()
+{
+    static const int recursions = CBOR_PARSER_MAX_RECURSIONS + 2;
+    QTest::addColumn<QByteArray>("data");
+
+    QTest::newRow("array") << QByteArray(recursions, '\x81') + '\x20';
+    QTest::newRow("_array") << QByteArray(recursions, '\x9f') + '\x20' + QByteArray(recursions, '\xff');
+
+    QByteArray data;
+    for (int i = 0; i < recursions; ++i)
+        data += "\xa1\x65Hello";
+    data += '\2';
+    QTest::newRow("map-recursive-values") << data;
+
+    data.clear();
+    for (int i = 0; i < recursions; ++i)
+        data += "\xbf\x65World";
+    data += '\2';
+    for (int i = 0; i < recursions; ++i)
+        data += "\xff";
+    QTest::newRow("_map-recursive-values") << data;
+
+    data = QByteArray(recursions, '\xa1');
+    data += '\2';
+    for (int i = 0; i < recursions; ++i)
+        data += "\x7f\x64quux\xff";
+    QTest::newRow("map-recursive-keys") << data;
+
+    data = QByteArray(recursions, '\xbf');
+    data += '\2';
+    for (int i = 0; i < recursions; ++i)
+        data += "\1\xff";
+    QTest::newRow("_map-recursive-keys") << data;
+
+    data.clear();
+    for (int i = 0; i < recursions / 2; ++i)
+        data += "\x81\xa1\1";
+    data += '\2';
+    QTest::newRow("mixed") << data;
+}
+
+void tst_Parser::recursionLimit()
+{
+    QFETCH(QByteArray, data);
+
+    CborParser parser;
+    CborValue first;
+    CborError err = cbor_parser_init(data.constData(), data.length(), 0, &parser, &first);
+    QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
+
+    // check that it is valid:
+    CborValue it = first;
+    {
+        QString dummy;
+        err = parseOne(&it, &dummy);
+        QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
+    }
+
+    it = first;
+    err = cbor_value_advance(&it);
+    QCOMPARE(int(err), int(CborErrorNestingTooDeep));
+
+    it = first;
+    if (cbor_value_is_map(&it)) {
+        CborValue dummy;
+        err = cbor_value_map_find_value(&it, "foo", &dummy);
+        QCOMPARE(int(err), int(CborErrorNestingTooDeep));
+    }
+}
+
 QTEST_MAIN(tst_Parser)
 #include "tst_parser.moc"