From a6b45a49d6ccdda930590f8a9f0501bf7c511627 Mon Sep 17 00:00:00 2001 From: Luiz Felipe Coelho Varanda Date: Tue, 13 Oct 2026 15:06:09 -0300 Subject: [PATCH 1/2] core: treat malformed definition/data messages as corruption, not crashes ULog() raised uncaught exceptions on malformed .ulg files: KeyError (_parse_nested_type, an ADD_LOGGED_MSG referencing an undefined/nested format), ValueError (_extract_type array spec), struct.error (the definitions phase only caught IndexError), and OverflowError (file.seek with a bogus appended offset from FLAG_BITS). Any service parsing an untrusted log was taken down unless every call was wrapped in try/except. - _read_file_definitions: move the header parse into the try and broaden the handler to (IndexError, ValueError, KeyError, OverflowError, struct.error), treating a malformed definition message as corruption like _read_file_data already does. The intentional "unknown incompatible flag" aborts (ValueError / NotImplementedError) are preserved and still propagate. - _read_file_data: broaden the per-message handler to also catch KeyError, ValueError and OverflowError. - _extract_type: reject an out-of-range array size (also prevents a range()-based resource-exhaustion DoS). - _load_file: guard the appended-data seek against a bogus offset. Fuzzing ULog() on mutated valid logs (seed=1, 4000 iterations) went from 10 distinct escaping exception sites to 3 - and those 3 are the library's intended invalid-header / incompatible-flag refusals. Fixes #127 --- pyulog/core.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/pyulog/core.py b/pyulog/core.py index 11e86d0..bf7e525 100644 --- a/pyulog/core.py +++ b/pyulog/core.py @@ -648,6 +648,9 @@ def _extract_type(field_str): else: b_pos = type_str.find(']') array_size = int(type_str[a_pos+1:b_pos]) + # malformed/huge array size from an untrusted log -> range() DoS + if array_size < 0 or array_size > 0xFFFF: + raise ValueError("invalid array size: %d" % array_size) type_name = type_str[:a_pos] return type_name, array_size, name_str @@ -872,8 +875,13 @@ def _load_file(self, log_file, message_name_filter_list, parse_header_only=False if self._debug: print('This file has data appended') for offset in self._appended_offsets: - self._read_file_data(message_name_filter_list, read_until=offset) - self._file_handle.seek(offset) + try: + self._read_file_data(message_name_filter_list, read_until=offset) + self._file_handle.seek(offset) + except (OverflowError, ValueError, OSError): + # bogus appended offset from a corrupt FLAG_BITS -> corruption + self._file_corrupt = True + break # read the whole file, or the rest if data appended self._read_file_data(message_name_filter_list) @@ -900,9 +908,9 @@ def _read_file_definitions(self): data = self._file_handle.read(3) if not data: break - header.initialize(data) - data = self._file_handle.read(header.msg_size) try: + header.initialize(data) + data = self._file_handle.read(header.msg_size) if header.msg_type == self.MSG_TYPE_INFO: msg_info = self._MessageInfo(data, header) self._msg_info_dict[msg_info.key] = msg_info.value @@ -958,7 +966,11 @@ def _read_file_definitions(self): # skipping the message self._file_handle.seek(-2-header.msg_size, 1) - except IndexError: + except (IndexError, ValueError, KeyError, OverflowError, struct.error) as exc: + # Keep the intentional incompatible-flag abort ("cannot parse the + # log"); treat any other failure as corruption, like _read_file_data. + if isinstance(exc, ValueError) and 'cannot parse the log' in str(exc): + raise if not self._file_corrupt: print("File corruption detected while reading file definitions!") self._file_corrupt = True @@ -1118,7 +1130,8 @@ def _read_file_data(self, message_name_filter_list, read_until=None): if self._has_sync: self._find_sync(header.msg_size) - except IndexError: + except (IndexError, KeyError, ValueError, OverflowError): + # malformed message (KeyError/ValueError/OverflowError) -> corruption if not self._file_corrupt: print("File corruption detected while reading file data!") self._file_corrupt = True From 0d297a971f7c6f5fb05e6c33cfc6cedf6de4f69f Mon Sep 17 00:00:00 2001 From: Luiz Felipe Coelho Varanda Date: Fri, 16 Oct 2026 16:28:00 -0300 Subject: [PATCH 2/2] core: add ULogException / ULogIncompatibleFlagException Use dedicated exceptions instead of matching the error message to tell the intentional incompatible-flag abort apart from file corruption. - Add pyulog/exceptions.py with ULogException (the file cannot be parsed) and its subclass ULogIncompatibleFlagException, both exported from pyulog. They live in their own module as core.py is right at pylint's max-module-lines. - Raise ULogException for an invalid file header (was TypeError) and ULogIncompatibleFlagException for an unknown incompatible flag (was ValueError / NotImplementedError). They are not caught by the corruption handlers, so the string check is gone. - Let UnicodeDecodeError propagate again: it is a ValueError subclass, so the broadened handlers turned string errors into corruption even with disable_str_exceptions=False. - Only guard the appended-data seek(), not _read_file_data(). - Add tests for both exceptions, corrupt definitions and strict string parsing. --- pyulog/__init__.py | 1 + pyulog/core.py | 23 ++++++++++---------- pyulog/exceptions.py | 15 +++++++++++++ test/test_ulog.py | 51 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 pyulog/exceptions.py diff --git a/pyulog/__init__.py b/pyulog/__init__.py index 7f0d5a2..a4a6d9b 100644 --- a/pyulog/__init__.py +++ b/pyulog/__init__.py @@ -1,3 +1,4 @@ """ Wrapper to include the main library modules """ from .core import ULog +from .exceptions import ULogException, ULogIncompatibleFlagException from . import px4 diff --git a/pyulog/core.py b/pyulog/core.py index bf7e525..0791851 100644 --- a/pyulog/core.py +++ b/pyulog/core.py @@ -5,6 +5,7 @@ import sys import contextlib import numpy as np +from .exceptions import ULogException, ULogIncompatibleFlagException #pylint: disable=too-many-instance-attributes, unused-argument, missing-docstring #pylint: disable=protected-access, too-many-branches @@ -101,6 +102,7 @@ def __init__(self, log_file, message_name_filter_list=None, disable_str_exceptio :param message_name_filter_list: list of strings, to only load messages with the given names. If None, load everything. :param disable_str_parser_exceptions: If True, ignore string parsing errors + :raises ULogException: if the file cannot be parsed (invalid header, incompatible flag) """ self._debug = False @@ -875,8 +877,8 @@ def _load_file(self, log_file, message_name_filter_list, parse_header_only=False if self._debug: print('This file has data appended') for offset in self._appended_offsets: + self._read_file_data(message_name_filter_list, read_until=offset) try: - self._read_file_data(message_name_filter_list, read_until=offset) self._file_handle.seek(offset) except (OverflowError, ValueError, OSError): # bogus appended offset from a corrupt FLAG_BITS -> corruption @@ -892,9 +894,9 @@ def _load_file(self, log_file, message_name_filter_list, parse_header_only=False def _read_file_header(self): header_data = self._file_handle.read(16) if len(header_data) != 16: - raise TypeError("Invalid file format (Header too short)") + raise ULogException("Invalid file format (Header too short)") if header_data[:7] != self.HEADER_BYTES: - raise TypeError("Invalid file format (Failed to parse header)") + raise ULogException("Invalid file format (Failed to parse header)") self._file_version, = struct.unpack('B', header_data[7:8]) if self._file_version > 1: print("Warning: unknown file version. Will attempt to read it anyway") @@ -949,10 +951,10 @@ def _read_file_definitions(self): unknown_incompat_flag_msg = \ "Unknown incompatible flag set: cannot parse the log" if self._incompat_flags[0] & ~1: - raise ValueError(unknown_incompat_flag_msg) + raise ULogIncompatibleFlagException(unknown_incompat_flag_msg) for i in range(1, 8): if self._incompat_flags[i]: - raise NotImplementedError(unknown_incompat_flag_msg) + raise ULogIncompatibleFlagException(unknown_incompat_flag_msg) else: if self._debug: @@ -966,11 +968,9 @@ def _read_file_definitions(self): # skipping the message self._file_handle.seek(-2-header.msg_size, 1) - except (IndexError, ValueError, KeyError, OverflowError, struct.error) as exc: - # Keep the intentional incompatible-flag abort ("cannot parse the - # log"); treat any other failure as corruption, like _read_file_data. - if isinstance(exc, ValueError) and 'cannot parse the log' in str(exc): - raise + except UnicodeDecodeError: + raise # strict string parsing (disable_str_exceptions=False) + except (IndexError, ValueError, KeyError, OverflowError, struct.error): if not self._file_corrupt: print("File corruption detected while reading file definitions!") self._file_corrupt = True @@ -1130,8 +1130,9 @@ def _read_file_data(self, message_name_filter_list, read_until=None): if self._has_sync: self._find_sync(header.msg_size) + except UnicodeDecodeError: + raise # strict string parsing (disable_str_exceptions=False) except (IndexError, KeyError, ValueError, OverflowError): - # malformed message (KeyError/ValueError/OverflowError) -> corruption if not self._file_corrupt: print("File corruption detected while reading file data!") self._file_corrupt = True diff --git a/pyulog/exceptions.py b/pyulog/exceptions.py new file mode 100644 index 0000000..11a486c --- /dev/null +++ b/pyulog/exceptions.py @@ -0,0 +1,15 @@ +""" Exceptions raised by pyulog """ + + +class ULogException(Exception): + """ + Raised if a ULog file cannot be parsed at all, e.g. because of an invalid header. + Corrupt messages are skipped instead, and reported via ULog.file_corruption. + """ + + +class ULogIncompatibleFlagException(ULogException): + """ + Raised if a ULog file has an unknown incompatible flag set, i.e. it uses a format + extension that this version of pyulog cannot parse. + """ diff --git a/test/test_ulog.py b/test/test_ulog.py index d135ef3..1272cd8 100644 --- a/test/test_ulog.py +++ b/test/test_ulog.py @@ -6,6 +6,7 @@ import inspect import unittest import tempfile +import struct from io import BytesIO from ddt import ddt, data @@ -89,4 +90,54 @@ def test_write_ulog_memory(self, base_name): else: assert copied_value == original_value + @staticmethod + def _build_ulog(*messages): + ''' + Build a minimal ULog file in memory from (msg_type, payload) tuples. + ''' + content = pyulog.ULog.HEADER_BYTES + b'\x01' + struct.pack('