fix: avoid unaligned reads in DNS and serializers

[RajaMuhammadAwais]

• I have signed the ntop Contributor License Agreement at https://github.com/ntop/legal/blob/main/individual-contributor-licence-agreement.md
• I have read the contributing guidelines at https://github.com/ntop/nDPI/blob/dev/CONTRIBUTING.md
• Documentation update is not applicable for this low-level portability and sanitizer bug fix. The changed behavior is documented in this pull request and in the related issue.

Link to the related issue:

#3213

Describe changes:

Summary

This pull request fixes undefined behavior and sanitizer failures caused by unaligned multi-byte reads in nDPI's DNS parsing and serializer code.

The issue is described in #3213. nDPI was reading 16-bit, 32-bit, and 64-bit values from byte buffers by casting byte pointers directly to integer pointers and dereferencing them. Such a cast requires the resulting address to satisfy the alignment requirement of the target integer type. Packet data and serialized data do not guarantee this alignment, particularly when a parser reaches an odd byte offset.

On x86-64, these reads may appear to work because the processor commonly permits unaligned access. However, the code still violates C alignment rules and UBSan correctly reports the operation as undefined behavior. On strict-alignment architectures such as ARM, MIPS, and some RISC-V configurations, the same access can fault natively. When nDPI is built with address and undefined-behavior sanitizers using -fno-sanitize-recover=all, the process terminates instead of continuing to process the packet.

Reproduced DNS failure

The official issue contains three 45-byte malformed DNS payloads. Each payload causes the DNS parser to walk to an odd offset before reading a 16-bit field.

The first trigger contains an invalid query count and causes the DNS query walker to continue after the only valid query. The second trigger reaches the EDNS OPT additional-record fields at an odd offset. The third trigger uses a compression-pointer byte pattern that causes the name walk to reach an odd offset.

The triggers were wrapped in UDP PCAP files and replayed through an independently built nDPI reader with address and undefined-behavior sanitizers enabled.

Before the fix, the exact observed diagnostic was:

protocols/dns.c:151:13: runtime error: load of misaligned address ... for type 'u_int16_t', which requires 2 byte alignment

All three official trigger PCAPs reproduced the failure on the clean latest upstream dev checkout. This confirms that the issue is deterministic and not based on an assumed code path.

Root cause in DNS parsing

The DNS helper get16() previously performed a direct cast and dereference similar to:

u_int16_t v = *(u_int16_t *)&payload[*i];
The DNS parser also contained direct pointer-cast reads for EDNS payload size, record data length, option code, option length, and the packet cache key. These accesses can receive odd offsets or otherwise unaligned addresses when processing attacker-controlled packet data.
The fix routes these reads through alignment-safe helpers. The existing network-byte-order conversion behavior is preserved. Only the way the bytes are loaded has changed.
**Root cause in serializer handling**
The serializer deserialization helpers previously read integer values using direct casts such as:
*s = ntohs(*((u_int16_t *) &deserializer->buffer.data[offset]));
The serializer object itself was exposed through an opaque character-only structure. The implementation then viewed that storage as ndpi_private_serializer. Because the public storage type did not preserve the alignment of the private implementation type, the full reader path could also trigger an alignment diagnostic when the object was accessed.
The fix makes the opaque serializer storage an alignment-preserving union while keeping its storage size and byte-buffer layout compatible with the implementation. Integer deserialization now uses the same alignment-safe raw-byte helpers for 16-bit, 32-bit, and 64-bit values.
Implementation details
The following changes are included:
The raw integer access helpers in src/include/ndpi_define.h.in now load 16-bit, 32-bit, and 64-bit values with memcpy. This preserves the existing raw host-endian semantics without requiring the source address to be aligned.
The get_u_int8_t helper remains a byte access because an 8-bit value has no alignment requirement.
The DNS dissector in src/lib/protocols/dns.c now uses the safe accessors for get16(), EDNS fields, and the packet cache key.
The serializer implementation in src/lib/ndpi_serializer.c now uses safe accessors for unsigned and signed 16-bit, 32-bit, and 64-bit deserialization.
The public ndpi_serializer storage in src/include/ndpi_typedefs.h now preserves the alignment required by ndpi_private_serializer while retaining the existing storage buffer.
The three official issue triggers were added to fuzz/corpus/fuzz_dns_parse/ so future DNS fuzzing retains coverage for the malformed inputs that originally exposed the problem.

[IvanNardi]

@RajaMuhammadAwais, I need to think a little bit about these changes...

@utoni, what do you think, especially about using memcpy also on x86-64?

[RajaMuhammadAwais]

Thanks @IvanNardi I understand the concern about applying memcpy-based loads on x86-64 as well. I checked the generated code with GCC -O2 on x86-64: the fixed-size 16-bit and 32-bit memcpy helpers are fully inlined as a single movzx/mov load, with no library call, so the generated code is equivalent to the direct load in this configuration. The main reason for using memcpy is to preserve defined C behavior for unaligned packet and serialized data, especially on strict-alignment architectures.

I agree that the final decision should be based on an nDPI-specific benchmark rather than this small compiler check. I can add a benchmark comparison for the relevant DNS and serializer paths, or the change can be restricted if the project prefers architecture-specific accessors. I will not modify the PR until you and utoni agree on the preferred approach.

[utoni]

@RajaMuhammadAwais, I need to think a little bit about these changes...

@utoni, what do you think, especially about using memcpy also on x86-64?

Fine, will be most likely optimized away on x64, and it's the right way to fix unaligned memory access

[IvanNardi]

@RajaMuhammadAwais, could you take a look at the failures with mingw and window, please?

Anyway, this PR will be merged after the next major release (expected this week)

[RajaMuhammadAwais]

@IvanNardi I investigated the three failing jobs and reproduced the exact TLS commands locally using a clean regenerated build. The parent commit produce identical results: tls-appdata.pcap reports 38 DPI packets and tls_verylong_certificate.pcap reports 32 DPI packets in both builds. The CI values of 86 and 48 therefore do not reproduce in a clean local PR build. I do not think the expected outputs should be updated. Could you please check whether the masan, s390x, and Ubuntu 26.04 jobs are reusing generated headers or stale build artifacts, or whether they are running a different build configuration? The source changes in this PR are limited to alignment-safe helpers and the CFFI serializer representation. No further code or test-output changes have been pushed.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[IvanNardi]

LGTM

[IvanNardi]

@utoni, are you fine with the final version?

[utoni]

LGTM