strncmp usage in ipfw

[Charles Swiger]

On Nov 29, 2004, at 2:25 PM, Brooks Davis wrote:
> char *var;
> if (!strncmp(var, "str", strlen(var)))
> 	...
> [ ... ]
> Was use of this idiom deliberate or accidental?

I can't speak for the author, but using the "n"-for-length variant of 
the string and printf() family of functions is considered an important 
saftey practice, especially for network/firewall/IDS software which may 
be exposed to externally generated data which contains deliberately 
malicious string lengths.

Since the topic came up, it's also potentially dangerous to write code 
like:

	char errstr[1024];
	/* ...intervening code...  */
	snprintf(errstr, 1024, "...");

...because people making changes to the code may change the size of 
errstr without changing the 1024 in the snprintf().  Using a macro for 
the size is better practice:

	#define ERRLEN (1024)
	char errstr[ERRLEN];
	/* ...intervening code...  */
	snprintf(errstr, ERRLEN, "...");

...but the strong recommendation I've seen is to always use sizeof():

	 snprintf(errstr, sizeof(errstr), ...)

This brings me back to your point with regard to partial matches; it 
might be the case that the IPFW code could use char arrays and 
sizeof(var) rather than char *'s and strlen(var) for some cases?  The 
former approach would not only address your concerns, Brooks, but also 
be faster.  Otherwise, I suspect that:

	char *var;
	if (!strncmp(var, "str", strlen(var)))
		...

...should become:

	#define STR "str"
	char *var;
	if (!strncmp(var, STR, sizeof(STR)))
		...

-- 
-Chuck