#1349 Failing SNMPv3 packets cause crashes

[Steinar H. Gunderson]

Hi,

I figured it was better to comment here than in an already cloesd ticket. The patch in #1214 is wrong; it assumes the callback data is always of the type synch_state, which is only the case for some of net-snmp's own tools. For all other users, e.g. the Perl module, it will end up writing spurious zeros into structures, corrupting pointers and causing a crash. This is typically evident when you try to query a device over SNMPv3 and that device isn't responding.

I believe the patch should just be reverted and the corresponding issue reopened in wait of a correct fix.

[Bart Van Assche]

Please provide a way to reproduce the behavior you ran into such that we can add a regression test.

[Steinar H. Gunderson]

It's tricky to make a simple repro, because it depends on a failure condition in the network.

[Bart Van Assche]

Can the reported crash be reproduced by sending SNMPv3 requests to a host on which no SNMP agent is running? If not, how about adding a mode in the Net-SNMP agent for selectively dropping packets?

[Steinar H. Gunderson]

I've tried reproducing by sending to a host that's down, but I haven't managed to make a simple repro that way, no.

In any case, I won't be spending much more time making a repro for this (it took forever to track down i the first place). The bug is clear enough when you look at the code, although of course a regression test would be nice to have.

[Bill Fenner]

Bart,

The reproduction is probably to modify snmpd in the way described in https://sourceforge.net/p/net-snmp/patches/1214/ - it triggers the code path in question.

Steinar,

Can you share a simple perl client that would experience this crash? The naive code "$sess = new SNMP::Session(...); print $sess->get(...)" does not crash against the above-modified server.

Can you try the attached patch?

[Steinar H. Gunderson]

Hi,

I believe you need to use Perl callbacks, ie. something like

my $cb = sub {
print "foo\n";
};
$sess->get(..., $cb);
while (1) {
SNMP::MainLoop();
}

Just print $sess->get(...) won't use a callback, since it's synchronous.

[Bill Fenner]

Yup, using the callback got me a crash. Interestingly, the crash was use-after-free, on the second packet of the exchange, not a corruption of the callback data structure after retries were completed. (Consistent with the reports at https://sourceforge.net/p/net-snmp/patches/1255/ / https://sourceforge.net/p/net-snmp/mailman/message/32268599/)

To be clear, I completely agree with your characterization of reaching into the magic as incorrect, but was wondering: did you have crashes and then inspect code and decide that this buggy code was at fault, or did you determine specifically that this code caused the problem?

[Steinar H. Gunderson]

I had crashes due to corrupted pointers and put a hardware watchpoint at the structure in question to figure out who was changing it. It triggered at the code in question, and removing it (by reverting the patch) fixed the issue.

[Bill Fenner]

I've updated net-snmp 5.4, 5.7 and 5.8 with an updated patch that uses a callback instead of reaching into the magic. Thanks for the precise problem report.