Skip to content

Commit

Permalink
Fix potential segfault with stream iterators.
Browse files Browse the repository at this point in the history
When trimming off the beginning of a stream, an existing iterator
could end up dereferencing its internal chunk pointer even if the
chunk now no longer existed. The issue was inside the
increment/decrement operations, which didn't check if the *current*
iterator offset was still valid (because only then the current chunk
is guaranteed to be valid too).

TODO:
    - Test for original problem missing, need to see if I can build one

Closes #1918.
  • Loading branch information
rsmmr committed Nov 19, 2024
1 parent d9c025d commit 3e703eb
Showing 1 changed file with 33 additions and 12 deletions.
45 changes: 33 additions & 12 deletions hilti/runtime/include/types/stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -717,13 +717,22 @@ class SafeConstIterator {
if ( ! n )
return;

_offset += n;
if ( _chain->isValid() ) {
const Chunk* hint = nullptr;

if ( _chain->inRange(_offset) )
hint = _chunk; // current chunk is still valid
else
hint = _chain->head(); // previous chunk was likely trimmed off, try new head

if ( ! (_chain && _chain->isValid()) )
return; // will be caught when dereferenced
_chunk = _chain->findChunk(_offset + n, hint); // null if we're pointing beyond the end now
}
else
// Invalid chain will trigger exception when dereferenced, but
// invalidate chunk to be safe.
_chunk = nullptr;

_chunk = _chain->findChunk(_offset, chunk());
// chunk will be null if we're pointing beyond the end.
_offset += n;
}

void _decrement(const integer::safe<uint64_t>& n) {
Expand All @@ -736,16 +745,28 @@ class SafeConstIterator {
if ( ! n )
return;

_offset -= n;
if ( _chain->isValid() ) {
const Chunk* hint = nullptr;

if ( _chunk && _offset > _chunk->offset() )
return; // fast-path, chunk still valid
if ( _chain->inRange(_offset) ) {
if ( _chunk && _chunk->inRange(_offset - n) ) {
_offset -= n;
return; // fast-path, inside still-valid chunk
}

if ( ! (_chain && _chain->isValid()) )
return; // will be caught when dereferenced
hint = _chunk; // current chunk is still valid
}
else
hint = _chain->head(); // previous chunk was likely trimmed off, try new head

_chunk = _chain->findChunk(_offset, _chunk);
// chunk will be null if we're pointing beyond the beginning.
_chunk = _chain->findChunk(_offset - n, hint); // null if we're pointing outside the chain now
}
else
// Invalid chain will trigger exception when dereferenced, but
// invalidate chunk to be safe.
_chunk = nullptr;

_offset -= n;
}

Byte _dereference() const {
Expand Down

0 comments on commit 3e703eb

Please sign in to comment.