Hello,
not being maintainer for ntopng I tried to reproduce the issue by
the steps below.
The crash happens because in MySQLDB.cpp this loop breaks only after
4 iterations instead of 2:
const u_int16_t ipvers[2] = {4, 6};
for (u_int16_t i = 0; i < sizeof(ipvers); i++){
Attached are two patches:
- 0001-Avoid-access-after-free.patch
(Unrelated to this bug, just received the output from valgrind.)
- 0002-Avoid-access-to-unintialized-memory.patch
(With this applied ntopng is not crashing for me; similar change
got applied upstream in
https://github.com/ntop/ntopng/commit/2d2e735c99064e8f45c38199e810b121d2b5f4b1 )
Was tested just as far as starting and stopping the service is involved.
Kind regards,
Bernhard
echo '-F="mysql;localhost;ntopng;flows;ntopng;simple"' >> /etc/ntopng.conf
mysql -u root -p
CREATE USER 'ntopng'@'localhost' IDENTIFIED BY 'simple';
create database ntopng;
GRANT ALL PRIVILEGES ON ntopng.* To 'ntopng'@'localhost' IDENTIFIED BY
'simple';
exit
systemctl start ntopng
Job for ntopng.service failed because a fatal signal was delivered causing the
control process to dump core.
See "systemctl status ntopng.service" and "journalctl -xe" for details.
journalctl -u ntopng
Mai 06 15:52:42 debian systemd[1]: Starting ntopng - High-Speed Web-based
Traffic Analysis and Flow Collection Tool...
Mai 06 15:52:42 debian ntopng[9957]: 06/May/2017 15:52:42 [Prefs.cpp:919]
Logging into /var/log/ntopng/ntopng.log
Mai 06 15:52:42 debian ntopng[9957]: 06/May/2017 15:52:42 [Ntop.cpp:1121]
Setting local networks to 127.0.0.0/8
Mai 06 15:52:42 debian ntopng[9957]: 06/May/2017 15:52:42 [Redis.cpp:92]
Successfully connected to redis 127.0.0.1:6379@0
Mai 06 15:52:42 debian ntopng[9957]: [NDPI] ndpi_init_protocol_defaults(missing
protoId=226) INTERNAL ERROR: not all protocols have been initialized
Mai 06 15:52:42 debian ntopng[9957]: 06/May/2017 15:52:42 [MySQLDB.cpp:495]
Attempting to connect to MySQL for interface dummy...
Mai 06 15:52:42 debian ntopng[9957]: 06/May/2017 15:52:42 [MySQLDB.cpp:535]
Succesfully connected to MySQL [localhost:ntopng] for interface dummy
Mai 06 15:52:45 debian ntopng[9957]: 06/May/2017 15:52:45 [MySQLDB.cpp:297]
MySQL schema update. Altering table flowsv4: renaming BYTES to IN_BYTES and
adding OUT_BYTES
Mai 06 15:52:46 debian ntopng[9957]: 06/May/2017 15:52:46 [MySQLDB.cpp:297]
MySQL schema update. Altering table flowsv6: renaming BYTES to IN_BYTES and
adding OUT_BYTES
Mai 06 15:52:48 debian systemd[1]: ntopng.service: Control process exited,
code=dumped status=11
Mai 06 15:52:48 debian systemd[1]: Failed to start ntopng - High-Speed
Web-based Traffic Analysis and Flow Collection Tool.
Mai 06 15:52:48 debian systemd[1]: ntopng.service: Unit entered failed state.
Mai 06 15:52:48 debian systemd[1]: ntopng.service: Failed with result
'core-dump'.
Mai 06 15:52:48 debian systemd[1]: ntopng.service: Service hold-off time over,
scheduling restart.
Mai 06 15:52:48 debian systemd[1]: Stopped ntopng - High-Speed Web-based
Traffic Analysis and Flow Collection Tool.
dmesg -T
[Sa Mai 6 15:52:47 2017] ntopng[9957]: segfault at 7fffc2e90000 ip
000055bfbe6c0ffe sp 00007fffc2e8cee0 error 4 in ntopng[55bfbe6ac000+8a000]
root@debian:/home/benutzer/debian/ntopng/ntopng/orig/ntopng-2.4+dfsg1/src#
coredumpctl gdb 9957
...
Core was generated by `/usr/sbin/ntopng /etc/ntopng.conf'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055bfbe6c0ffe in MySQLDB::MySQLDB (this=0x55bfc0eec850,
_iface=<optimized out>) at src/MySQLDB.cpp:307
307 exec_sql_query(&mysql, sql, true, true);
(gdb) bt
#0 0x000055bfbe6c0ffe in MySQLDB::MySQLDB (this=0x55bfc0eec850,
_iface=<optimized out>) at src/MySQLDB.cpp:307
#1 0x000055bfbe6e35f0 in NetworkInterface::NetworkInterface
(this=0x55bfbffa7fb0, name=0x55bfbe715310 "dummy") at
src/NetworkInterface.cpp:133
#2 0x000055bfbe6c6042 in Prefs::add_default_interfaces (this=<optimized out>)
at src/Prefs.cpp:1059
#3 0x000055bfbe6bc7d4 in main (argc=2, argv=0x7fffc2e8f298) at src/main.cpp:117
root@debian:/home/benutzer/debian/ntopng/ntopng/orig/ntopng-2.4+dfsg1/src#
valgrind /usr/sbin/ntopng /etc/ntopng.conf
==10143== Memcheck, a memory error detector
==10143== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==10143== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==10143== Command: /usr/sbin/ntopng /etc/ntopng.conf
==10143==
06/May/2017 16:27:49 [Prefs.cpp:919] Logging into /var/log/ntopng/ntopng.log
06/May/2017 16:27:49 [Ntop.cpp:1121] Setting local networks to 127.0.0.0/8
06/May/2017 16:27:49 [Redis.cpp:92] Successfully connected to redis
127.0.0.1:6379@0
[NDPI] ndpi_init_protocol_defaults(missing protoId=226) INTERNAL ERROR: not all
protocols have been initialized
06/May/2017 16:27:49 [MySQLDB.cpp:495] Attempting to connect to MySQL for
interface dummy...
06/May/2017 16:27:50 [MySQLDB.cpp:535] Succesfully connected to MySQL
[localhost:ntopng] for interface dummy
==10143== Invalid read of size 8
==10143== at 0x616E301: mysql_num_rows (client.c:4561)
==10143== by 0x11C1AD: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool,
bool) (MySQLDB.cpp:593)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== Address 0x144527a8 is 8 bytes inside a block of size 208 free'd
==10143== at 0x4C2CDDB: free (vg_replace_malloc.c:530)
==10143== by 0x11C1A5: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool,
bool) (MySQLDB.cpp:592)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== Block was alloc'd at
==10143== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==10143== by 0x61A7D95: my_malloc (my_malloc.c:101)
==10143== by 0x616C1D5: mysql_store_result (client.c:4094)
==10143== by 0x11C190: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool,
bool) (MySQLDB.cpp:589)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Use of uninitialised value of size 8
==10143== at 0x7B0A16B: _itoa_word (_itoa.c:179)
==10143== by 0x7B0E869: vfprintf (vfprintf.c:1636)
==10143== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==10143== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==10143== by 0x11CF2A: snprintf (stdio2.h:65)
==10143== by 0x11CF2A: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Conditional jump or move depends on uninitialised value(s)
==10143== at 0x7B0A175: _itoa_word (_itoa.c:179)
==10143== by 0x7B0E869: vfprintf (vfprintf.c:1636)
==10143== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==10143== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==10143== by 0x11CF2A: snprintf (stdio2.h:65)
==10143== by 0x11CF2A: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Conditional jump or move depends on uninitialised value(s)
==10143== at 0x7B0E971: vfprintf (vfprintf.c:1636)
==10143== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==10143== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==10143== by 0x11CF2A: snprintf (stdio2.h:65)
==10143== by 0x11CF2A: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Conditional jump or move depends on uninitialised value(s)
==10143== at 0x7B0D831: vfprintf (vfprintf.c:1636)
==10143== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==10143== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==10143== by 0x11CF2A: snprintf (stdio2.h:65)
==10143== by 0x11CF2A: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Conditional jump or move depends on uninitialised value(s)
==10143== at 0x7B0D8B2: vfprintf (vfprintf.c:1636)
==10143== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==10143== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==10143== by 0x11CF2A: snprintf (stdio2.h:65)
==10143== by 0x11CF2A: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143==
==10143== Invalid read of size 2
==10143== at 0x11CFFE: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:307)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== Address 0xfff001000 is not stack'd, malloc'd or (recently) free'd
==10143==
==10143==
==10143== Process terminating with default action of signal 11 (SIGSEGV)
==10143== Access not within mapped region at address 0xFFF001000
==10143== at 0x11CFFE: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:307)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*)
(NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== If you believe this happened as a result of a stack
==10143== overflow in your program's main thread (unlikely but
==10143== possible), you can try to increase the size of the
==10143== main thread stack using the --main-stacksize= flag.
==10143== The main thread stack size used in this run was 8388608.
==10143==
==10143== HEAP SUMMARY:
==10143== in use at exit: 15,932,693 bytes in 95,935 blocks
==10143== total heap usage: 103,101 allocs, 7,166 frees, 22,180,739 bytes
allocated
==10143==
==10143== LEAK SUMMARY:
==10143== definitely lost: 87 bytes in 3 blocks
==10143== indirectly lost: 0 bytes in 0 blocks
==10143== possibly lost: 1,544 bytes in 20 blocks
==10143== still reachable: 15,931,062 bytes in 95,912 blocks
==10143== of which reachable via heuristic:
==10143== length64 : 34,152 bytes in 17 blocks
==10143== newarray : 1,536 bytes in 16 blocks
==10143== suppressed: 0 bytes in 0 blocks
==10143== Rerun with --leak-check=full to see details of leaked memory
==10143==
==10143== For counts of detected and suppressed errors, rerun with: -v
==10143== Use --track-origins=yes to see where uninitialised values come from
==10143== ERROR SUMMARY: 28739 errors from 7 contexts (suppressed: 0 from 0)
Speicherzugriffsfehler (Speicherabzug geschrieben)
From d105ca470feb89ac9ca4589600dd2a78df3e50cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bernhard=20=C3=9Cbelacker?= <bernha...@mailbox.org>
Date: Sat, 6 May 2017 16:31:36 +0200
Subject: Avoid access after free.
Found while investigating for https://bugs.debian.org/859653
==10143== Invalid read of size 8
==10143== at 0x616E301: mysql_num_rows (client.c:4561)
==10143== by 0x11C1AD: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool, bool) (MySQLDB.cpp:593)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*) (NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== Address 0x144527a8 is 8 bytes inside a block of size 208 free'd
==10143== at 0x4C2CDDB: free (vg_replace_malloc.c:530)
==10143== by 0x11C1A5: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool, bool) (MySQLDB.cpp:592)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*) (NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
==10143== Block was alloc'd at
==10143== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==10143== by 0x61A7D95: my_malloc (my_malloc.c:101)
==10143== by 0x616C1D5: mysql_store_result (client.c:4094)
==10143== by 0x11C190: MySQLDB::exec_sql_query(st_mysql*, char*, bool, bool, bool) (MySQLDB.cpp:589)
==10143== by 0x11CF4F: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:295)
==10143== by 0x13F5EF: NetworkInterface::NetworkInterface(char const*) (NetworkInterface.cpp:133)
==10143== by 0x122041: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==10143== by 0x1187D3: main (main.cpp:117)
---
src/MySQLDB.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/MySQLDB.cpp b/src/MySQLDB.cpp
index b9a1337..9148f25 100644
--- a/src/MySQLDB.cpp
+++ b/src/MySQLDB.cpp
@@ -589,8 +589,8 @@ int MySQLDB::exec_sql_query(MYSQL *conn, char *sql,
if((result = mysql_store_result(&mysql)) == NULL)
rc = 0; // unable to retrieve the result but still the query succeded
else{
- mysql_free_result(result);
rc = mysql_num_rows(result);
+ mysql_free_result(result);
}
}
--
2.11.0
From 3b8a1f8f11021bdd8319c175fd9743c6427c5541 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bernhard=20=C3=9Cbelacker?= <bernha...@mailbox.org>
Date: Sat, 6 May 2017 16:36:18 +0200
Subject: Avoid access to unintialized memory.
Found while investigating for https://bugs.debian.org/859653
==14371== Use of uninitialised value of size 8
==14371== at 0x7B0A16B: _itoa_word (_itoa.c:179)
==14371== by 0x7B0E869: vfprintf (vfprintf.c:1636)
==14371== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==14371== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==14371== by 0x11D2EA: snprintf (stdio2.h:65)
==14371== by 0x11D2EA: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:294)
==14371== by 0x1496CF: NetworkInterface::NetworkInterface(char const*) (NetworkInterface.cpp:133)
==14371== by 0x122791: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==14371== by 0x1188F3: main (main.cpp:117)
==19200== Use of uninitialised value of size 8
==19200== at 0x7B0A16B: _itoa_word (_itoa.c:179)
==19200== by 0x7B0E869: vfprintf (vfprintf.c:1636)
==19200== by 0x7BBC8F5: __vsnprintf_chk (vsnprintf_chk.c:63)
==19200== by 0x7BBC857: __snprintf_chk (snprintf_chk.c:34)
==19200== by 0x11D474: snprintf (stdio2.h:65)
==19200== by 0x11D474: MySQLDB::MySQLDB(NetworkInterface*) (MySQLDB.cpp:321)
==19200== by 0x14980F: NetworkInterface::NetworkInterface(char const*) (NetworkInterface.cpp:133)
==19200== by 0x1228D1: Prefs::add_default_interfaces() (Prefs.cpp:1059)
==19200== by 0x1188F3: main (main.cpp:117)
---
src/MySQLDB.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/MySQLDB.cpp b/src/MySQLDB.cpp
index 9148f25..faf087a 100644
--- a/src/MySQLDB.cpp
+++ b/src/MySQLDB.cpp
@@ -289,7 +289,7 @@ MySQLDB::MySQLDB(NetworkInterface *_iface) : DB(_iface) {
// Move column BYTES to BYTES_IN and add BYTES_OUT
// note that this operation will arbitrarily move the old BYTES contents to BYTES_IN
const u_int16_t ipvers[2] = {4, 6};
- for (u_int16_t i = 0; i < sizeof(ipvers); i++){
+ for (u_int16_t i = 0; i < sizeof(ipvers)/sizeof(*ipvers); i++){
snprintf(sql, sizeof(sql), "SHOW COLUMNS FROM `%sv%hu` LIKE 'BYTES'",
ntop->getPrefs()->get_mysql_tablename(), ipvers[i]);
if(exec_sql_query(&mysql, sql, true, true) > 0){
@@ -309,7 +309,7 @@ MySQLDB::MySQLDB(NetworkInterface *_iface) : DB(_iface) {
}
// Modify database engine to MyISAM (that is much faster in non-transactional environments)
- for (u_int16_t i = 0; i < sizeof(ipvers); i++){
+ for (u_int16_t i = 0; i < sizeof(ipvers)/sizeof(*ipvers); i++){
snprintf(sql, sizeof(sql),
"SELECT 1 "
"FROM information_schema.TABLES "
--
2.11.0
