snmpd: do not touch uninitialized memory

[Alexander Sabourenkov]

Dumping string-type data would spew contents of uninitialized memory because
ber_get_string does no effort to zero-terminate its result.

Index: snmpe.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.25
diff -u snmpe.c
--- snmpe.c 16 Dec 2009 22:17:53 -0000 1.25
+++ snmpe.c 18 Dec 2009 12:15:24 -0000
@@ -433,9 +433,11 @@
root->be_type == SNMP_T_IPADDR) {
fprintf(stderr, "addr %s\n",
inet_ntoa(*(struct in_addr *)buf));
- } else
- fprintf(stderr, "string \"%s\"\n",
- root->be_len ? buf : "");
+ } else {
+ fwrite("string \"", 8, 1, stderr);
+ fwrite(buf, root->be_len, 1, stderr);
+ fwrite("\"\n", 2, 1, stderr);
+ }
break;
case BER_TYPE_NULL: /* no payload */
case BER_TYPE_EOC:

--

./lxnt

[Jacek Masiulaniec]

On Fri, Dec 18, 2009 at 03:26:14PM +0300, Alexander Sabourenkov wrote:
> Dumping string-type data would spew contents of uninitialized memory
> because ber_get_string does no effort to zero-terminate its result.

Please submit diffs that apply to -current.

I admit ignorance of SNMP internals but by inspecting the code I
am not convinced this is the right fix. Before working around
missing NUL terminator it must become clear why is it missing in
the first place.

Look at buf_read_element. The BER_TYPE_OCTETSTRING is always
NUL-terminated. Why is the terminator missing few lines later?

Then look at mps_setstr. It strdups BER_TYPE_OCTETSTRING. If NUL
was always missing, then use of strdup would be incorrect. Is it?

Jacek