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/git.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/git.c')
| -rw-r--r-- | src/lib/protocols/git.c | 7 |
1 files changed, 5 insertions, 2 deletions
diff --git a/src/lib/protocols/git.c b/src/lib/protocols/git.c index 22fc6f76e..2d194be81 100644 --- a/src/lib/protocols/git.c +++ b/src/lib/protocols/git.c @@ -47,8 +47,11 @@ void ndpi_search_git(struct ndpi_detection_module_struct *ndpi_struct, u_int32_t git_pkt_len; memcpy(&len, &pp[offset], 4), len[4] = 0; - sscanf(len, "%x", &git_pkt_len); - + if(sscanf(len, "%x", &git_pkt_len) != 1) { + found_git = 0; + break; + } + if((payload_len < git_pkt_len) || (git_pkt_len == 0 /* Bad */)) { found_git = 0; break; |