Package: src:exabgp Version: 5.0.8-1 User: [email protected] Usertags: python3.15 Tags: patch, ftbfs, forky, sid Severity: important
Hi! While rebuilding the python related packages against the Python 3.15rc2 version we found that exabgp fails to build from source [1]. The error is: struct.error: unpack requires a buffer of 2 bytes The fix is to backport upstream commit f3ba9c75f "fix: check a message is long enough before reading its fixed header" [2]. I've applied the upstream fix in the sandbox [3] to verify that it builds successfully, please consider applying the patch to support the upcoming 3.15 version. Setting the severity to important for now. Once Python 3.15 is released, it will be added to python3-defaults and this bug will become release critical. Happy hacking, [1]: https://debusine.debian.net/debian/r-python-python3.15/artifact/4467110/ [2]: https://github.com/Exa-Networks/exabgp/commit/f3ba9c75f55547af917727d835b6a3c55ca40a4c [3]: https://debusine.debian.net/debian/r-python-python3.15/ -- "Can you imagine what I would do if I could do all I can?" -- Sun Tzu Saludos /\/\ /\ >< `/
From: Thomas Mangin <[email protected]> Date: Fri, 21 Aug 2026 14:52:08 +0100 Subject: fix: check a message is long enough before reading its fixed header Four places read a fixed number of bytes off a peer supplied buffer with nothing checking the bytes were there, so the failure was a raw Python exception out of the message parser rather than a NOTIFICATION: Update.split computed `length = len(data)` and then never consulted it, unpacking the two mandatory 2 byte length fields of RFC 4271 4.3 regardless. One stray byte raised struct.error. Open read version, AS, hold time and identifier without checking the ten bytes RFC 4271 4.2 requires were present. Notification read data[0] and data[1] off a body which may be empty. Message.unpack fell through to klass_unknown, bound to Exception unless exabgp.bgp.message.unknown is imported, and nothing imports it, so it CONSTRUCTED an Exception and RETURNED it as though it were a message. Type 0 is in CODE.MESSAGES, so it passes the reactor's own check and reached that line from the wire. A NOTIFICATION too short to carry a code now closes the connection without a reply: RFC 4271 6.5 says an error found while processing one is indicated by closing, not by answering with another. Returning a Notification rather than a Notify is what does that here, and the test asserts the difference, which isinstance cannot since Notify subclasses Notification. An unrecognised message type is now 1/3 Bad Message Type, which RFC 4271 6.1 requires, where the reactor used 1/0 Unspecific. That changes what a peer receives, so it is in CHANGELOG and the functional fixture is updated. The catch-all further down keeps 1/0, because there we genuinely do not know what went wrong. WHY NONE OF THIS WAS CAUGHT The fuzz suite asserted, 7 times: except Exception as e: assert not isinstance(e, (SystemExit, KeyboardInterrupt)) SystemExit and KeyboardInterrupt derive from BaseException, so `except Exception` can never bind either. The assertion could not fail whatever the code did. Worse, the test named "UPDATE message parsing doesn't crash on random data" called Update.unpack(env, data, None, None): four arguments to Message.unpack(message, data, direction, negotiated). `message` was an Env object, no registered type matched, and every example raised "unknown message type" before touching the data. It had never parsed an UPDATE. The OPEN test had the same defect and an except which swallowed everything without asserting at all. Both now call the decoder they name, and the assertions say which exceptions each site may raise. Verified by disabling the UPDATE gate and watching the fuzz test go red, which it did not do before. Only 2 of the 7 sites are reached in practice; the other 5 never raise. Said plainly in the helper rather than left to look like coverage. The same sweep found three configuration parser crashes. unescape stepped past a trailing backslash and read off the end; it yielded bytes into a join over strings for \uXXXX, so that escape raised TypeError and had never once produced a character; and Tokeniser assigned Tokeniser._off, the function, where it meant Tokeniser._off(), the empty iterator it returns, so after a configuration file failed to open the next read raised TypeError. _next was also initialised to None where every other path uses [], which line[:-1] turns into a TypeError on the first call. ValueError is what tokens() already raises for bad syntax, so that is what these raise now: 6000 random texts produce ValueError and nothing else. Each gate verified by breaking it: five separate breakages, each failing a different test, and the good-path tests fail if a gate refuses valid input. --- src/exabgp/bgp/message/update/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/exabgp/bgp/message/update/__init__.py b/src/exabgp/bgp/message/update/__init__.py index 2ed7f1bcd..a435f84e3 100644 --- a/src/exabgp/bgp/message/update/__init__.py +++ b/src/exabgp/bgp/message/update/__init__.py @@ -86,6 +86,13 @@ class Update(Message): def split(data): length = len(data) + # RFC 4271 4.3: the body opens with two 2 byte length fields. length was + # computed and never consulted, so a body shorter than the first of them + # left struct.error here rather than a NOTIFICATION: one stray byte on + # the wire was enough + if length < UPDATE_ATTR_LENGTH_HEADER_SIZE: + raise Notify(3, 1, 'invalid UPDATE, %d bytes cannot hold the two length fields' % length) + len_withdrawn = unpack('!H', data[0:UPDATE_WITHDRAWN_LENGTH_OFFSET])[0] withdrawn = data[UPDATE_WITHDRAWN_LENGTH_OFFSET : len_withdrawn + UPDATE_WITHDRAWN_LENGTH_OFFSET] @@ -93,6 +100,10 @@ class Update(Message): raise Notify(3, 1, 'invalid withdrawn routes length, not enough data available') start_attributes = len_withdrawn + UPDATE_ATTR_LENGTH_HEADER_SIZE + # the withdrawn routes are accounted for, but the total path attribute + # length which follows them still has to be present + if length < start_attributes: + raise Notify(3, 1, 'invalid UPDATE, no room for the total path attribute length') len_attributes = unpack('!H', data[len_withdrawn + UPDATE_WITHDRAWN_LENGTH_OFFSET : start_attributes])[0] start_announced = len_withdrawn + len_attributes + UPDATE_ATTR_LENGTH_HEADER_SIZE attributes = data[start_attributes:start_announced]
