diff options
| author | Ivan Nardi <12729895+IvanNardi@users.noreply.github.com> | 2021-10-27 09:23:07 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-10-27 09:23:07 +0200 |
| commit | 6edb7bedd7b5c1eb0060f5d503f9e37a6d58c086 (patch) | |
| tree | 28dedbd73b956f6a96a9c19dc35b4c4b1f9dc4d7 /src/lib/protocols/tls.c | |
| parent | c4eebaaf5eaf54bb5f2656a504a4485ace1d0da4 (diff) | |
Avoid overwriting valid protocol in `ndpi_detection_giveup` (#1360)
We should avoid updating any valid protocol in `ndpi_detection_giveup`; we
should try to find a proper classification only if the flow is still
completely unclassified.
For example in the attached pcap there is a valid TLS session, recognized
as such by TLS dissector. However, the `ndpi_detection_giveup`function
updates it to "HTTP/TLS" (!?) simply because the server port is 80.
Note that the real issue is not the wrong classification, but the
wrong access to `flow->protos` union. If we already set some fields of
`flow->protos` and we change the protocol in `ndpi_detection_giveup`, we
might end up freeing some invalid pointers in `ndpi_free_flow_data`
(no wonder this issue has been found while fuzzing #1354)
Fix GIT and TLS dissectors (issues found by CI fuzzer)
Diffstat (limited to 'src/lib/protocols/tls.c')
| -rw-r--r-- | src/lib/protocols/tls.c | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/src/lib/protocols/tls.c b/src/lib/protocols/tls.c index aafa89048..6be5740e0 100644 --- a/src/lib/protocols/tls.c +++ b/src/lib/protocols/tls.c @@ -1951,7 +1951,7 @@ int processClientServerHello(struct ndpi_detection_module_struct *ndpi_struct, printf("Client TLS [SIGNATURE_ALGORITHMS: %s]\n", ja3.client.signature_algorithms); #endif } else if(extension_id == 16 /* application_layer_protocol_negotiation */ && - offset+extension_offset < total_len) { + offset+extension_offset+1 < total_len) { u_int16_t s_offset = offset+extension_offset; u_int16_t tot_alpn_len = ntohs(*((u_int16_t*)&packet->payload[s_offset])); char alpn_str[256]; |