| Commit message (Collapse) | Author | Age |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remove some unreached/duplicated code.
Add error checking for `atoi()` calls.
About `isdigit()` and similar functions. The warning reported is:
```
Negative Character Value help
isdigit() is invoked here with an argument of signed type char, but only
has defined behavior for int arguments that are either representable
as unsigned char or equal to the value of macro EOF(-1).
Casting the argument to unsigned char will avoid the undefined behavior.
In a number of libc implementations, isdigit() is implemented using lookup
tables (arrays): passing in a negative value can result in a read underrun.
```
Switching to our macros fix that.
Add a check to `check_symbols.sh` to avoid using the original functions
from libc.
|
|
|
| |
See: b08c787fe
|
|
|
|
|
|
| |
1) Public API/headers in `src/include/` [as it has always been]
2) Private API/headers in `src/lib/`
Try to keep the "ndpi_" prefix only for the public functions
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
All dissector callbacks should not be exported by the library; make static
some other local functions.
The callback logic in `ndpiReader` has never been used.
With internal libgcrypt, `gcry_control()` should always return no
errors.
We can check `categories` length at compilation time.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The application may enable only some protocols.
Disabling a protocol means:
*) don't register/use the protocol dissector code (if any)
*) disable classification by-port for such a protocol
*) disable string matchings for domains/certificates involving this protocol
*) disable subprotocol registration (if any)
This feature can be tested with `ndpiReader -B list_of_protocols_to_disable`.
Custom protocols are always enabled.
Technically speaking, this commit doesn't introduce any API/ABI
incompatibility. However, calling `ndpi_set_protocol_detection_bitmask2()`
is now mandatory, just after having called `ndpi_init_detection_module()`.
Most of the diffs (and all the diffs in `/src/lib/protocols/`) are due to
the removing of some function parameters.
Fix the low level macro `NDPI_LOG`. This issue hasn't been detected
sooner simply because almost all the code uses only the helpers `NDPI_LOG_*`
|
|
|
|
|
|
|
| |
The return value of the extra-dissection callback indicates if the extra
dissection needs to be called again.
In the HTTP cose, this setting to NULL of the callabck is wrong since we
stop extra dissection only if we have a hostname *and* a return code.
|
|
|
|
|
|
|
|
| |
See 95e16872.
After c0732eda, we can safely remove the protocol list from
`ndpi_process_extra_packet()`.
The field `flow->check_extra_packets` is redundant; remove it.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
```
protocols/snmp_proto.c:77:23: runtime error: signed integer overflow: 6 + 2147483647 cannot be represented in type 'int'
#0 0x52f69e in ndpi_search_snmp ndpi/src/lib/protocols/snmp_proto.c:77:23
#1 0x4c5347 in check_ndpi_detection_func ndpi/src/lib/ndpi_main.c:5211:4
#2 0x4c5591 in ndpi_check_flow_func ndpi/src/lib/ndpi_main.c:0
#3 0x4c8903 in ndpi_detection_process_packet ndpi/src/lib/ndpi_main.c:6145:15
#4 0x4b3712 in LLVMFuzzerTestOneInput ndpi/fuzz/fuzz_process_packet.c:29:5
[...]
```
Found by oss-fuzzer.
See: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49057
|
| |
|
| |
|
|
|
| |
Increase max number of flows handled during fuzzing
|
| |
|
|
|
|
|
| |
Similar to the error fixed in 4775be3d
Found by oss-fuzz.
See: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46713
|
|
|
|
|
|
|
| |
* Removed Visual Studio leftovers. Maintaining an autotools project with VS integration requires some additional overhead.
Signed-off-by: Toni Uhlig <matzeton@googlemail.com>
Signed-off-by: lns <matzeton@googlemail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
```
==19724==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60e00000045e at pc 0x5620b8b3d3cc bp 0x7ffe0fda6b50 sp 0x7ffe0fda6310
READ of size 2 at 0x60e00000045e thread T0
#0 0x5620b8b3d3cb in __interceptor_strncpy (/home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet_with_main+0x63f3cb) (BuildId: ee53ff920c8cd4c226d8520a0d4846d8864726b6)
#1 0x5620b8d9b69c in strncpy_lower /home/ivan/svnrepos/nDPI/src/lib/protocols/kerberos.c:208:4
#2 0x5620b8d995a0 in krb_parse /home/ivan/svnrepos/nDPI/src/lib/protocols/kerberos.c:316:5
#3 0x5620b8d97a90 in ndpi_search_kerberos /home/ivan/svnrepos/nDPI/src/lib/protocols/kerberos.c:687:12
#4 0x5620b8bcef35 in check_ndpi_detection_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:4996:4
#5 0x5620b8bd1be8 in check_ndpi_udp_flow_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5072:10
#6 0x5620b8bd159c in ndpi_check_flow_func /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5105:12
#7 0x5620b8be323a in ndpi_detection_process_packet /home/ivan/svnrepos/nDPI/src/lib/ndpi_main.c:5924:15
#8 0x5620b8b8f7e0 in LLVMFuzzerTestOneInput /home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet.c:24:3
#9 0x5620b8b8fd1b in main /home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet.c:84:17
#10 0x7f45b32b90b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
#11 0x5620b8acf47d in _start (/home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet_with_main+0x5d147d) (BuildId: ee53ff920c8cd4c226d8520a0d4846d8864726b6)
0x60e00000045e is located 0 bytes to the right of 158-byte region [0x60e0000003c0,0x60e00000045e)
allocated by thread T0 here:
#0 0x5620b8b5283e in malloc (/home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet_with_main+0x65483e) (BuildId: ee53ff920c8cd4c226d8520a0d4846d8864726b6)
#1 0x5620b8b8fc86 in main /home/ivan/svnrepos/nDPI/fuzz/fuzz_process_packet.c:70:17
#2 0x7f45b32b90b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
```
```
protocols/kerberos.c:79:52: runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
```
Found by oss-fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46670
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46636
|
|
|
|
|
|
| |
* This is a quick fix, the Kerberos protocol dissector requires some refactoring effort.
Signed-off-by: Toni Uhlig <matzeton@googlemail.com>
Signed-off-by: lns <matzeton@googlemail.com>
|
|
|
|
|
|
| |
Detected by oss-fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43823
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43921
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43925
|
|
|
|
| |
Detected by oss-fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43677
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As a general rule, the higher the confidence value, the higher the
"reliability/precision" of the classification.
In other words, this new field provides an hint about "how" the flow
classification has been obtained.
For example, the application may want to ignore classification "by-port"
(they are not real DPI classifications, after all) or give a second
glance at flows classified via LRU caches (because of false positives).
Setting only one value for the confidence field is a bit tricky: more
work is probably needed in the next future to tweak/fix/improve the logic.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We can write to `flow->protos` only after a proper classification.
This issue has been found in Kerberos, DHCP, HTTP, STUN, IMO, FTP,
SMTP, IMAP and POP code.
There are two kinds of fixes:
* write to `flow->protos` only if a final protocol has been detected
* move protocol state out of `flow->protos`
The hard part is to find, for each protocol, the right tradeoff between
memory usage and code complexity.
Handle Kerberos like DNS: if we find a request, we set the protocol
and an extra callback to further parsing the reply.
For all the other protocols, move the state out of `flow->protos`. This
is an issue only for the FTP/MAIL stuff.
Add DHCP Class Identification value to the output of ndpiReader and to
the Jason serialization.
Extend code coverage of fuzz tests.
Close #1343
Close #1342
|
|
|
|
|
|
|
|
|
|
|
|
| |
ndpi_finalize_initialization(). (#1334)
* fixed several memory errors (heap-overflow, unitialized memory, etc)
* ability to build fuzz_process_packet with a main()
allowing to replay crash data generated with fuzz_process_packet
by LLVMs libfuzzer
* temporarily disable fuzzing if `tests/do.sh`
executed with env FUZZY_TESTING_ENABLED=1
Signed-off-by: Toni Uhlig <matzeton@googlemail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are no real reasons to embed `struct ndpi_packet_struct` (i.e. "packet")
in `struct ndpi_flow_struct` (i.e. "flow"). In other words, we can avoid
saving dissection information of "current packet" into the "flow" state,
i.e. in the flow management table.
The nDPI detection module processes only one packet at the time, so it is
safe to save packet dissection information in `struct ndpi_detection_module_struct`,
reusing always the same "packet" instance and saving a huge amount of memory.
Bottom line: we need only one copy of "packet" (for detection module),
not one for each "flow".
It is not clear how/why "packet" ended up in "flow" in the first place.
It has been there since the beginning of the GIT history, but in the original
OpenDPI code `struct ipoque_packet_struct` was embedded in
`struct ipoque_detection_module_struct`, i.e. there was the same exact
situation this commit wants to achieve.
Most of the changes in this PR are some boilerplate to update something
like "flow->packet" into something like "module->packet" throughout the code.
Some attention has been paid to update `ndpi_init_packet()` since we need
to reset some "packet" fields before starting to process another packet.
There has been one important change, though, in ndpi_detection_giveup().
Nothing changed for the applications/users, but this function can't access
"packet" anymore.
The reason is that this function can be called "asynchronously" with respect
to the data processing, i.e in context where there is no valid notion of
"current packet"; for example ndpiReader calls it after having processed all
the traffic, iterating the entire session table.
Mining LRU stuff seems a bit odd (even before this patch): probably we need
to rethink it, as a follow-up.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fix all the warnings.
Getting rid of "-Wno-unused-parameter" is quite complex because some
parameters usage depends on compilation variable (i.e.
`--enable-debug-messages`).
The "-Werror" flag has been added only in Travis builds to avoid
breaking the builds to users using uncommon/untested
OS/compiler/enviroment.
Tested on:
* x86_64; Ubuntu 20.04; gcc 7,8,9,10,11; clang 7,8,9,10,11,12
* x86_64; CentOS 7.7; gcc 4.8.5 (with "--disable-gcrypt" flag)
* Raspberry 4; Debian 10.10; gcc 8.3.0
|
| |
|
| |
|
| |
|
|
|
|
| |
Signed-off-by: Toni Uhlig <matzeton@googlemail.com>
|
| |
|
|
|
|
| |
Fixes warning
|
| |
|
| |
|
| |
|
|
|
|
|
| |
After leaving kerberos code, the original packet may be processed from
other dissector (i.e. TLS)
|
| |
|
| |
|
| |
|
|\ |
|
| |
| |
| |
| | |
Minor TLS cleanup
|
|/ |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
Fixes
(gdb) f 0
101 if(cname_str[cname_len-1] == '$') {
(gdb) p cname_len
$3 = 0
|
| |
|
| |
|
| |
|
| |
|