Bugs found by out scanner

[yuanqilbx]

Hi, we developed a taint analysis based static analysis tool named Vanguard. It could prognosis potential vulnerabilities by identifying security-sensitive operations (e.g. divide-zero, mod-zero, array-index-access, and sensitive function calls) without proper checks for their operands.

Some code locations are listed in the following. We think these locations maybe bugs after our manual analysis. Please check them, and add precondition checks if necessary.

Bug-1:Array index bound 1.in function ikev1_d_print,print-isakmp.c#L1818

		ND_PRINT(" proto=%s", PROTOIDSTR(proto));

Array expression: protoidstr[(proto)] needs bound checking: proto < 5;

2.in function ikev2_auth_print,print-isakmp.c#L2330

		ND_PRINT(" [|%s]", NPSTR(tpay));

Array expression: npstr[(tpay)] needs bound checking: tpay < 49;

3.in function ikev1_n_print, print-isakmp.c#L1725

		ND_PRINT(" proto=%s", PROTOIDSTR(proto));

Array expression: protoidstr[(proto)] needs bound checking: proto < 5;

4.in function ikev2_t_print, print-isakmp.c#L1959

ND_PRINT(" #%u type=%s id=%u ", tcount,
			  STR_OR_ID(t_type, ikev2_t_type_map),
			  t_id);

Array expression: ikev2_t_type_map[(t_type)] needs bound checking: t_type < 6;

5.in function ikev1_id_print, print-isakmp.c#L1374

			ND_PRINT(" idtype=%s", STR_OR_ID(type, ipsecidtypestr));

Array expression: ipsecidtypestr[(type)] needs bound checking: type < 12;

6.in function ikev2_nonce_print,print-isakmp.c#L2359

		ND_PRINT(" [|%s]", NPSTR(tpay));

Array expression: npstr[(tpay)] needs bound checking: tpay < 49;

7.in function ikev2_ke_print,print-isakmp.c#L2187

		ND_PRINT(" [|%s]", NPSTR(tpay));

Array expression: npstr[(tpay)] needs bound checking: tpay < 49;

Bug2:Mem_op_argument 1.in function MakeFilename,tcpdump.c#L833

		strncpy(buffer, filename, PATH_MAX + 1);

[strncpy] is a memory operation function using tainted data: [buffer ]

[mcr]

We are in the process of replacing many of these macros. I'd like to come back to your feedback before our 5.0.0 release in June.

[DimitriPapadopoulos]

All issues involving STR_OR_ID or macros that use STR_OR_ID (PROTOIDSTR or NPSTR) look like false positives, because STR_OR_ID itself does bound checking: https://github.com/the-tcpdump-group/tcpdump/blob/a5211097d02144ddd11f71f07c01697750c69fd9/print-isakmp.c#L724-L725

[DimitriPapadopoulos]

@yuanqilbx About this piece of code: https://github.com/the-tcpdump-group/tcpdump/blob/a5211097d02144ddd11f71f07c01697750c69fd9/tcpdump.c#L833

Have you found occurrences where the size of buffer is less than PATH_MAX + 1, or where strlen(filename) > PATH_MAX? Why do you think there's a bug?