cbor/bpv7: store native field values and harden CBOR containers - #1
Closed
polybassa wants to merge 49 commits into
Closed
cbor/bpv7: store native field values and harden CBOR containers#1polybassa wants to merge 49 commits into
polybassa wants to merge 49 commits into
Conversation
to get scapy to capture packets on both 32-bit and 64-bit machines. The bpf_ts structure contains two 64-bit fields: https://github.com/freebsd/freebsd-src/blob/aea4240ef5834fb4a47f80c659c80f902cb4bb06/sys/net/bpf.h#L206-L209 ``` struct bpf_ts { bpf_int64 bt_sec; /* seconds */ bpf_u_int64 bt_frac; /* fraction */ }; ``` and it has been this way since it was introduced in freebsd/freebsd-src@547d94b back in 2010. ``` Introduce new time stamp 'struct bpf_ts' and header 'struct bpf_xhdr'. The new time stamp has both 64-bit second and fractional parts. bpf_xhdr has this time stamp instead of 'struct timeval' for bh_tstamp. The new structures let us use bh_tstamp of same size on both 32-bit and 64-bit platforms without adding additional shims for 32-bit binaries. ``` It was tested on FreeBSD 14.4-RELEASE/i386 and 15.1-RELEASE-p2/amd64. AI-Assisted: no
* Update inet6.py for Destination Option Extension Headers can show weird behavious. Linux's sk_buff considers the IPv6 Payload to be either TCP, UDP or ICMP. It does not consider Extension Headers to be the payload. Following similar architecture, This small modification let's packet flow with Destination Option on both, request and response packets be captured as well. * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add tests for IPv6 with Destination Option header (hashret/answers) * Fix flake8 errors: trailing whitespace and inline comment formatting --------- Co-authored-by: Guillaume Valadon <guillaume@valadon.net> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
) byteCount is a byte count, but FieldListField.count_from expects an element count. For the arrays whose elements are 16-bit registers that made the field consume twice as many items as the PDU actually holds, so when several ADUs arrived in one buffer the array swallowed the header of the following one and returned wrong values. Parsing two concatenated Read Input Registers responses gave registerVal [0x1111, 0x2222, 0x3333, 1, 0, 9], the trailing three shorts being the next ADU's transId, protoId and len, and the second ADU was left unrecoverable. No exception was raised. Divide byteCount by the element size for those arrays. The three arrays of byte-sized elements were already correct and are untouched. Read FIFO Queue is a special case: its byteCount also covers the FIFOCount field, so use FIFOCount, which is the element count the spec provides. ModbusPDU01ReadCoilsResponse additionally derived from _ModbusPDUNoPayload, whose extract_padding discards everything after the PDU, so a following ADU was dropped rather than mangled. Its counterpart ModbusPDU02ReadDiscreteInputsResponse already derives from Packet; match it. Reported and diagnosed by Insanitree in secdev#4096. Fixes: secdev#4096 AI-Assisted: yes (Claude Fable 5, via Claude Code)
* Parse ATT_Read_By_Group_Type_Response fields Parses the attribute data in ATT_Read_By_Group_Type_Response packets. AI-Assisted: no * Add tests for ATT packets Adds tests for ATT_Read_By_Type_Request and ATT_Read_By_Group_Type_Request. AI-Assisted: no * Simplify ATT handle variable parsing Removes "val_length" slots from ATT_Handle_Variable and ATT_Group_Handle_Variable and calculates the value length from the parent packet instead. AI-Assisted: no
PyPy has been broken for several months now. There is a regression in the interpreter that brears our CI. DIsable for now
Clarify guidelines for AI-assisted reports and PRs.
) NBNS_ADD_ENTRY never ended itself, so the first entry took the rest of RDATA as its payload and PacketListField stopped after one pass. A name with several owners came back as one entry followed by a Raw, and nbns_resolve() handed the caller only the first address. NBNSNodeStatusResponseService already yields Padding for the same reason. AI-Assisted: no
Adds the following BLE ATT packet types and tests: - ATT_Read_Multiple_Variable_Request - ATT_Read_Multiple_Variable_Response - ATT_Multiple_Handle_Value_Notification - ATT_Handle_Value_Confirmation - ATT_Signed_Write_Command AI-Assisted: yes (Claude Sonnet 5)
This is still in beta on Windows, and looks buggy right now. Let's wait a bit before enabling this by default. AI-Assisted: no
TCPSession splits multiple NBT messages out of one TCP segment with 'packet /= sub_packet'. That operator copies both operands, so a segment holding n messages copies chains of 1, 2, ... n-1 messages and the total work grows with the square of the message count. A 1,440-byte payload can hold 360 four-byte keepalives. Going from 80 to 160 to 320 messages moved processing from 42.81 to 186.15 to 766.25 ms. sub_packet is built immediately above and is not shared, so it does not need a defensive copy. add_payload appends it directly, which also makes the following _strip_padding act on the object that was appended rather than on a discarded clone. A valid two-message benchmark measured 182,735.4 ns/call before and 167,463.3 ns/call after, within the machine's noise. AI-Assisted: yes (GPT-5.6-Cyber)
…v#5103) Each parsed option sliced the remaining unread metadata, so a capture with many small valid options made rdpcap() cost time quadratic in the option count. A 524,408-byte file with 65,536 comments took 251 ms to read while an equal-size file with eight comments took under one millisecond. Doubling the file took 1.24 s. Frames need not be large; these files hold one 34-byte Ethernet/IP frame and the rest is capture metadata. Walk the buffer by offset instead of reslicing it. A five-point growth run is classified linear after the change, and the valid-option benchmark went from 13,458.4 to 9,185.9 ns/call. AI-Assisted: yes (GPT-5.6-Cyber)
…5102) HPACK Huffman decoding rebuilt the remaining bit string on each symbol, making a valid Huffman-coded value cost time quadratic in its length. Decode over the input bytes instead. Full-EOS rejection and the RFC 7541 padding checks are unchanged; integer callers convert to bytes once. Valid short Huffman parses took 50,945.5 ns unmodified and 42,879.6 ns with this change. AI-Assisted: yes (GPT-5.6-Cyber)
AI-Assisted: no
…se (secdev#5101) * ldap: bound the bytes materialized while reassembling a search response Segmented LDAP search responses are re-parsed from the start of the buffer on each arriving segment, so a valid multi-message response makes session-aware capture stall well past the point the data is complete. Bound the total bytes materialized per reassembly attempt. A segmented search response still returns LDAP_SearchResponseResultDone. A small valid multi-read response took 124,933.9 ns/call unmodified and 121,541.6 ns/call with this change. AI-Assisted: yes (GPT-5.6-Cyber) * ldap: address review — rename to tcp_min_len, use pop Renames the metadata key to `tcp_min_len` and drops the separate `del`, as requested. The key is read with `get` and only popped once the length is satisfied. Popping on the skip path clears the guard, so the next segment reassembles again and the cost stays quadratic — measured doubling ratios 2.17 / 2.34 / 2.61 with an unconditional pop, against 1.94 / 1.99 / 2.00 this way. AI-Assisted: yes (GPT-5.6-Cyber)
count_from was given the string 'length' instead of a callable, so any MD-Type 2 packet carrying context headers raised "TypeError: 'str' object is not callable" in PacketListField.getfield. Only MD-Type 1 and the empty MD-Type 2 case worked, which is all the tests covered. count_from is also the wrong parameter here. RFC 8300 section 2.2 makes the base header length a count of 4-byte words, not of context headers, and section 2.5.1 makes the TLV length a count of metadata bytes padded up to a word boundary. Both are length_from now, and the length field is computed with length_of instead of count_of. NSHTLV also needs extract_padding, otherwise the first TLV swallows the ones behind it. AI-Assisted: yes (Claude Code)
Both fields passed a field class where a packet class is expected. STDOBJREF wrapped UUIDField in a PacketField, so dissection died in PacketField.m2i with "UUIDField.__init__() missing 1 required positional argument: 'default'". _make_le already special-cases UUIDField, so the field was meant to be used directly. OpcDaFack passed IntField to a PacketListField, which failed silently: selackLen=2 dissected into a single Raw covering both entries. A list of plain fields is FieldListField. AI-Assisted: yes (Claude Code)
…le (secdev#5094) Extended_Advertise_Set does not override extract_padding, so the first set claims the remaining sets as its payload and PacketListField stops after one entry. A command with num_sets=2 dissected into len(sets)==1, with the second set left in sets[0].payload as Raw. HCI_LE_Meta_Extended_Advertising_Report already does this in the same module. AI-Assisted: yes (Claude Code)
AI-Assisted: yes (GPT-5.6-Cyber)
AI-Assisted: yes (GPT-5.5-Cyber)
AI-Assisted: yes (GPT-5.5-Cyber)
Fix and regression test for GHSA-92qm-jfgf-qqxg. AI-Assisted: yes (GPT-5.6-Cyber) Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com>
Net.__init__ accepted a scope argument but never read it, so the
documented Net("224.0.0.1", scope=conf.iface) form returned a Net whose
scope was None. Because __iter__ copies self.scope onto the addresses it
yields, such a Net produced unscoped addresses, and packets built from it
left through the default interface rather than the requested one. Net6
inherits __init__ and behaved the same way.
Pass the argument to ScopedIP, which resolves the interface and keeps the
precedence where an inline % overrides it.
The existing tests could not catch this because Net.__eq__ does not look
at scope, so the new test asserts on .scope itself.
AI-Assisted: yes (Claude Opus 5)
Co-authored-by: Dylan Pulver <dylanpulver@users.noreply.github.com>
AI-Assisted: yes (GPT-5.6-Cyber)
MQTTPublish.msgid and MQTTPublish.value read QOS/len from the underlayer without a None check, so MQTTPublish().show() raised AttributeError. Guard both callbacks so a standalone publish packet can be displayed. Fixes secdev#5071 AI-Assisted: yes (Claude) Signed-off-by: SeaStar Deng <172368758@qq.com>
* sessions: bound missing ranges in TCP reassembly AI-Assisted: yes (GPT-5.6-Cyber) * sessions: make the reassembly gap bound a StringBuffer parameter Adds max_gap=MTU to the constructor and warns when data is dropped. AI-Assisted: yes (GPT-5.6-Cyber)
* tftp: extend multi-block read results in place AI-Assisted: yes (GPT-5.6-Cyber) * tftp: keep the read buffer as a bytearray throughout Addresses review feedback on secdev#5110: rather than switching self.res between bytes and bytearray, initialise it as a bytearray and convert once when the automaton returns. Removes the isinstance branching; the accumulate line goes back to a plain +=. AI-Assisted: yes (GPT-5.6-Sol)
Fix and regression test for GHSA-qphr-qxg4-fw3q. AI-Assisted: yes (GPT-5.6-Cyber) Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com>
Fix and regression test for GHSA-xhxx-36p6-j6jw. AI-Assisted: yes (GPT-5.6-Cyber) Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com>
* A zero XCP MAX_CTO response stops later response matching Fix and regression test for GHSA-m8ff-3rv8-hfvq. AI-Assisted: yes (GPT-5.6-Cyber) * Apply suggestion from @gpotter2 Co-authored-by: Gabriel <10530980+gpotter2@users.noreply.github.com> * Apply suggestion from @gpotter2 Co-authored-by: Gabriel <10530980+gpotter2@users.noreply.github.com> --------- Co-authored-by: Clinton Thomas <1033162+KernelClint@users.noreply.github.com> Co-authored-by: Nils Weiss <nils@dissecto.com>
AI-Assisted: no
AI-Assisted: no
AI-Assisted: no
CBOR packet fields now keep Python-native internals instead of CBOR_Object wrappers, with explicit build/dissect item accounting so required and conditional fields cannot silently shift array positions. Indefinite-length decoding, AI 31 handling, CRC auto-calculation, DTN time, IPN EID normalization, and BundleV7.validate() follow the PR secdev#5075 fix plan; incomplete BPSec stubs are removed from this change. AI-Assisted: yes Co-authored-by: Cursor <cursoragent@cursor.com>
Required fields always report min_items=1, CBOR_NO_ITEM separates sequence end from null, semantic tags and packet children use real item counts, and BPv7 CRC helpers share inferred type-code context. Harden maps/simple values/UTF-8 chunks/ranges, DTN/EID guards, and add focused UTS coverage for the review matrix. AI-Assisted: yes Co-authored-by: Cursor <cursoragent@cursor.com>
Respect parent array item budgets, require one-item packet cardinality on dissect, enforce required/present map members, add value-level APIs for ARRAY_OF/SEQUENCE_OF of semantic tags, and cover the findings with UTS. AI-Assisted: yes Co-authored-by: Cursor <cursoragent@cursor.com>
Harden optional lookahead, map key identity, nested packet lifecycle, received-wire CRC checks, and BPv7 validation. Apply low-risk encode simplifications (join-based codecs, cached map keys, shared helpers) and invalidate CBOR raw_packet_cache when nested fields change so dissected bundles rebuild correctly. AI-Assisted: yes Co-authored-by: Cursor <cursoragent@cursor.com>
Harden CBOR optional absence, array item reservation, mutable defaults, CRC single-build paths, deterministic encoding checks, safedec nesting, and linear decoding. Add follow-up review campaigns plus an opt-in cbor2 differential suite for PR secdev#5075. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
Store wire arity alongside the RFC 9758 allocator/node/service triple so decode/re-encode keeps the original packed or explicit SSP array shape, per Brian's PR secdev#5075 review note. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
Make CBOR sentinels copy-stable, honor zero-budget optionals, skip opaque BTSD as CBOR, recognize null IPN EIDs, and cover the remaining merge-gate gaps with dedicated latest-review unit tests. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap long CBOR/BPv7 expressions to the 88-character limit and restore a single-line Repology packaging badge so CI style checks pass. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent large binary64 validation crashes, enforce indefinite-map key order, parse/format LocalNode "!" per RFC 9758, and reject composed invalid Null-IPN service numbers. Fold PR review CBOR UTS into cbor.uts alongside the dedicated cbor2 interop campaign. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CBOR packet fields now keep Python-native internals instead of CBOR_Object wrappers, with explicit build/dissect item accounting so required and conditional fields cannot silently shift array positions. Indefinite-length decoding, AI 31 handling, CRC auto-calculation, DTN time, IPN EID normalization, and BundleV7.validate() follow the PR secdev#5075 fix plan; incomplete BPSec stubs are removed from this change.
AI-Assisted: yes