Fix illegal stream content access through invalid view.

When a view's beginning ended up outside of the stream's valid range (e.g., because the stream has been trimmed in the meantime), we wouldn't catch that on access and attempt to access data that was no longer valid. This commit extends the safety checks accordingly. Closes #1795.
zeek · Jul 22, 2024 · 3199cc6 · 3199cc6
1 parent 4a1b43e
commit 3199cc6
Show file tree

Hide file tree

Showing 9 changed files with 247 additions and 174 deletions.
diff --git a/hilti/runtime/include/types/stream.h b/hilti/runtime/include/types/stream.h
@@ -1366,9 +1366,9 @@ class View final {
     }
 
     /**
-     * Advances the view's starting position by a given number of stream.
+     * Advances the view's starting position by a given number of stream bytes.
      *
-     * @param i the number of stream to advance.
+     * @param i the number of stream bytes to advance.
      */
     View advance(const integer::safe<uint64_t>& i) const { return View(begin() + i, _end); }
 
@@ -1568,11 +1568,11 @@ class View final {
     }
 
     void _ensureValid() const {
-        //        if ( ! _begin.chain() )
-        //            throw InvalidIterator("view has invalid beginning");
+        if ( ! _begin.isValid() )
+            throw InvalidIterator("view has invalid beginning");
 
-        //        if ( ! _begin.isValid() )
-        //            throw InvalidIterator("view has invalid beginning");
+        if ( (! _begin.isUnset()) && _begin.offset() < _begin.chain()->offset() )
+            throw InvalidIterator("view starts before available range");
 
         if ( _end && ! _end->isValid() )
             throw InvalidIterator("view has invalid end");

diff --git a/hilti/runtime/src/tests/stream.cc b/hilti/runtime/src/tests/stream.cc
@@ -1009,7 +1009,7 @@ TEST_CASE("View") {
     }
 
     SUBCASE("extract") {
-        const auto s = Stream("1234567890"_b);
+        auto s = Stream("1234567890"_b);
         const auto v = s.view();
 
         SUBCASE("1") {
@@ -1035,6 +1035,19 @@ TEST_CASE("View") {
             CHECK_THROWS_WITH_AS(Stream().view().extract(dst, sizeof(dst)), "end of stream view", const WouldBlock&);
         }
 
+        SUBCASE("trimmed too much") {
+            s.trim(s.begin() + 5);
+            Byte dst[1] = {'0'};
+            CHECK_THROWS_WITH_AS(v.extract(dst, sizeof(dst)), "view starts before available range",
+                                 const InvalidIterator&);
+        }
+
+        SUBCASE("beginning invalid") {
+            s = Stream(); // let view expire
+            Byte dst[1] = {'0'};
+            CHECK_THROWS_WITH_AS(v.extract(dst, sizeof(dst)), "view has invalid beginning", const InvalidIterator&);
+        }
+
         SUBCASE("gaps") {
             auto s = Stream();
             SUBCASE("just gap") {

diff --git a/spicy/runtime/src/parser.cc b/spicy/runtime/src/parser.cc
@@ -69,9 +69,10 @@ void detail::printParserState(std::string_view unit_id, const hilti::rt::ValueRe
             begin_ = hilti::rt::fmt("%" PRId64, begin->offset());
 
         return hilti::rt::fmt("- state: type=%s input=\"%s%s\" stream=%p offsets=%" PRId64 "/%s/%" PRId64 "/%" PRId64
-                              " chunks=%d frozen=%s mode=%s trim=%s lah=%" PRId64 " lah_token=\"%s%s\" recovering=%s",
+                              "/%" PRId64 " chunks=%d frozen=%s mode=%s trim=%s lah=%" PRId64
+                              " lah_token=\"%s%s\" recovering=%s",
                               unit_id, input_data, input_dots, data.get(), data->begin().offset(), begin_,
-                              cur.begin().offset(), data->end().offset(), data->numberOfChunks(),
+                              cur.begin().offset(), data->end().offset(), cur.end().offset(), data->numberOfChunks(),
                               (data->isFrozen() ? "yes" : "no"), literal_mode, (trim ? "yes" : "no"), lah_str, lah_data,
                               lah_dots, (error.has_value() ? "yes" : "no"));
     };

diff --git a/tests/Baseline/spicy.types.unit.size-inside-size/output b/tests/Baseline/spicy.types.unit.size-inside-size/output
@@ -0,0 +1,32 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[spicy] Test::Messages
+[spicy]   Test::Message
+[spicy]     len = 8
+[spicy]       Test::TryInner
+[spicy]         Test::Inner
+[spicy]           len = 157
+[spicy]           magic = \\x91
+[spicy]         _anon = [$len=157, $magic=b"\\x91"]
+[spicy]       Test::TryInner
+[spicy]         Test::Inner
+[spicy]           len = 93
+[spicy]           magic = 
+[spicy]         _anon = [$len=93, $magic=b""]
+[spicy]       Test::TryInner
+[spicy]         Test::Inner
+[spicy]           len = 46
+[spicy]           magic = \\xd8
+[spicy]         _anon = [$len=46, $magic=b"\\xd8"]
+[spicy]       Test::TryInner
+[spicy]         Test::Inner
+[spicy]           len = 34
+[spicy]           magic = n
+[spicy]         _anon = [$len=34, $magic=b"n"]
+[spicy]       Test::TryInner
+[spicy]         Test::Inner
+[spicy]           len = 11
+[spicy]           magic = o\\xeb
+[spicy]         _anon = [$len=11, $magic=b"o\\xeb"]
+[spicy]       _anon_2 = [[], [], [], [], []]
+[spicy]   Test::Message
+[error] terminating with uncaught exception of type hilti::rt::InvalidIterator: view starts before available range (<...>/size-inside-size.spicy:18:8-18:12)
diff --git a/tests/Baseline/spicy.types.unit.synchronize-on-gap/gap_between_matches.log b/tests/Baseline/spicy.types.unit.synchronize-on-gap/gap_between_matches.log
@@ -1,10 +1,10 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[spicy-verbose] - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
+[spicy-verbose] - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
 [spicy-verbose] - parsing production: Unit: sync__X1 -> xs
-[spicy-verbose]   - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
+[spicy-verbose]   - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
 [spicy-verbose]   - parsing production: While: xs  -> while(<look-ahead-found>): _anon
-[spicy-verbose]     - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="A" recovering=no
-[spicy-verbose]     - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="A" recovering=no
+[spicy-verbose]     - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="A" recovering=no
+[spicy-verbose]     - state: type=sync::X1 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="A" recovering=no
 [spicy-verbose]     - parsing production: Ctor: _anon -> /(A|B|C)/ (regexp) (container 'xs')
 [spicy-verbose]       - consuming look-ahead token
 [spicy-verbose]       - trimming input
@@ -14,22 +14,22 @@
 [spicy-verbose]     resuming after insufficient input, now have 1024 for stream 0xXXXXXXXX
 [spicy-verbose]     failed to parse list element, will try to synchronize at next possible element
 [spicy-verbose]     - trimming input
-[spicy-verbose]     - state: type=sync::X1 input="" stream=0xXXXXXXXX offsets=1025/0/1025/1025 chunks=0 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
+[spicy-verbose]     - state: type=sync::X1 input="" stream=0xXXXXXXXX offsets=1025/0/1025/1025/1025 chunks=0 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
 [spicy-verbose]     suspending to wait for more input for stream 0xXXXXXXXX, currently have 0
 [spicy-verbose]     resuming after insufficient input, now have 3 for stream 0xXXXXXXXX
-[spicy-verbose]     - state: type=sync::X1 input="XBC" stream=0xXXXXXXXX offsets=1025/0/1025/1028 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
-[spicy-verbose]     - state: type=sync::X1 input="XBC" stream=0xXXXXXXXX offsets=1025/0/1025/1028 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
+[spicy-verbose]     - state: type=sync::X1 input="XBC" stream=0xXXXXXXXX offsets=1025/0/1025/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
+[spicy-verbose]     - state: type=sync::X1 input="XBC" stream=0xXXXXXXXX offsets=1025/0/1025/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
 [spicy-verbose]     - trimming input
-[spicy-verbose]     - state: type=sync::X1 input="BC" stream=0xXXXXXXXX offsets=1026/0/1026/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="B" recovering=yes
+[spicy-verbose]     - state: type=sync::X1 input="BC" stream=0xXXXXXXXX offsets=1026/0/1026/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="B" recovering=yes
 [spicy-verbose]     successfully synchronized
-[spicy-verbose]     - state: type=sync::X1 input="BC" stream=0xXXXXXXXX offsets=1026/0/1026/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="B" recovering=no
+[spicy-verbose]     - state: type=sync::X1 input="BC" stream=0xXXXXXXXX offsets=1026/0/1026/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="B" recovering=no
 [spicy-verbose]     - parsing production: Ctor: _anon -> /(A|B|C)/ (regexp) (container 'xs')
 [spicy-verbose]       - consuming look-ahead token
 [spicy-verbose]       - trimming input
 [spicy-verbose]       - trimming input
 [spicy-verbose]     - got container item
-[spicy-verbose]     - state: type=sync::X1 input="C" stream=0xXXXXXXXX offsets=1027/0/1027/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="C" recovering=no
-[spicy-verbose]     - state: type=sync::X1 input="C" stream=0xXXXXXXXX offsets=1027/0/1027/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="C" recovering=no
+[spicy-verbose]     - state: type=sync::X1 input="C" stream=0xXXXXXXXX offsets=1027/0/1027/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="C" recovering=no
+[spicy-verbose]     - state: type=sync::X1 input="C" stream=0xXXXXXXXX offsets=1027/0/1027/1028/1028 chunks=1 frozen=no mode=default trim=yes lah=1 lah_token="C" recovering=no
 [spicy-verbose]     - parsing production: Ctor: _anon -> /(A|B|C)/ (regexp) (container 'xs')
 [spicy-verbose]       - consuming look-ahead token
 [spicy-verbose]       - trimming input

diff --git a/tests/Baseline/spicy.types.unit.synchronize-on-gap/gap_while_matching.log b/tests/Baseline/spicy.types.unit.synchronize-on-gap/gap_while_matching.log
@@ -1,27 +1,27 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[spicy-verbose] - state: type=sync::X2 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
+[spicy-verbose] - state: type=sync::X2 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
 [spicy-verbose] - parsing production: Unit: sync__X2 -> xs_2
-[spicy-verbose]   - state: type=sync::X2 input="A" stream=0xXXXXXXXX offsets=0/0/0/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
+[spicy-verbose]   - state: type=sync::X2 input="A" stream=0xXXXXXXXX offsets=0/0/0/1/1 chunks=1 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=no
 [spicy-verbose]   - parsing production: While: xs_2 -> while(<look-ahead-found>): _anon_2
 [spicy-verbose]     suspending to wait for more input for stream 0xXXXXXXXX, currently have 1
 [spicy-verbose]     resuming after insufficient input, now have 1025 for stream 0xXXXXXXXX
 [spicy-verbose]     failed to parse list element, will try to synchronize at next possible element
 [spicy-verbose]     - trimming input
-[spicy-verbose]     - state: type=sync::X2 input="" stream=0xXXXXXXXX offsets=1025/0/1025/1025 chunks=0 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
+[spicy-verbose]     - state: type=sync::X2 input="" stream=0xXXXXXXXX offsets=1025/0/1025/1025/1025 chunks=0 frozen=no mode=default trim=yes lah=n/a lah_token="n/a" recovering=yes
 [spicy-verbose]     suspending to wait for more input for stream 0xXXXXXXXX, currently have 0
 [spicy-verbose]     resuming after insufficient input, now have 2 for stream 0xXXXXXXXX
-[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1025/0/1025/1027 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=yes
+[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1025/0/1025/1027/1027 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=yes
 [spicy-verbose]     successfully synchronized
-[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1025/0/1025/1027 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
+[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1025/0/1025/1027/1027 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
 [spicy-verbose]     - parsing production: Ctor: _anon_2 -> /AB/ (regexp) (container 'xs')
 [spicy-verbose]       - consuming look-ahead token
 [spicy-verbose]       - trimming input
 [spicy-verbose]       - trimming input
 [spicy-verbose]     - got container item
 [spicy-verbose]     suspending to wait for more input for stream 0xXXXXXXXX, currently have 0
 [spicy-verbose]     resuming after insufficient input, now have 2 for stream 0xXXXXXXXX
-[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1027/0/1027/1029 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
-[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1027/0/1027/1029 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
+[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1027/0/1027/1029/1029 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
+[spicy-verbose]     - state: type=sync::X2 input="AB" stream=0xXXXXXXXX offsets=1027/0/1027/1029/1029 chunks=1 frozen=no mode=default trim=yes lah=2 lah_token="AB" recovering=no
 [spicy-verbose]     - parsing production: Ctor: _anon_2 -> /AB/ (regexp) (container 'xs')
 [spicy-verbose]       - consuming look-ahead token
 [spicy-verbose]       - trimming input