Bug 7698 - Assert causes smbd to panic on invalid NetBIOS session request.
Assert causes smbd to panic on invalid NetBIOS session request.
Product: Samba 3.5
Classification: Unclassified
Component: File services
All All
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2010-09-26 04:59 UTC by Jeremy Allison
Modified: 2010-10-06 14:11 UTC (History)
0 users

See Also:

git-am fix for 3.5.next (12.65 KB, patch)
2010-09-26 06:51 UTC, Jeremy Allison
vl: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2010-09-26 04:59:26 UTC
Found by the CodeNomicon test suites at the SNIA plugfest.


If an invalid NetBIOS session request is received the code in name_len() in libsmb/nmblib.c can hit an assert.

Comment 1 Jeremy Allison 2010-09-26 06:51:33 UTC
Created attachment 5984 [details]
git-am fix for 3.5.next

Volker, please check and re-assign to Karolin if you're ok. This is the fix I put into master and v3-6-test, modified for 3.5.next.

Comment 2 Volker Lendecke 2010-09-27 05:45:27 UTC
Comment on attachment 5984 [details]
git-am fix for 3.5.next

Jeremy, before I bless this I'd like to see the sniff that made smbd run into the assert. This packet-length twiddling is a bit too fiddly to get right from just looking at the code.


Comment 3 Jeremy Allison 2010-09-27 07:43:52 UTC
Unfortunately it was from a proprietary app (the CodeNomicon suite). I do have the data that their tool creates describing the flaw they injected into the packet, but in order to reproduce I'll have to hand craft a NetBIOS type 81 packet with invalid name. I'll code that into smbtorture (in source3) as a regression test (shouldn't be too hard). I'll update the bug when the test is in place.

Comment 4 Jeremy Allison 2010-09-27 20:59:58 UTC
We now have a regression test in bin/smbtorture - BAD-NBT-SESSION
added with d7c09f312ee326c3108c7d06bc9c7390861d8552 in master.

Running this test crashes 3.5.x, now passes against v3-6-test and master.

Comment 5 Volker Lendecke 2010-10-03 05:09:40 UTC
Comment on attachment 5984 [details]
git-am fix for 3.5.next

Patch fixes the problem. Although to be honest, the patch is more difficult to understand than necessary, as it could have been a bit better partitioned as a patch. It puts everything into one basked which should be avoided IMVHO
Comment 6 Karolin Seeger 2010-10-06 14:11:16 UTC
Pushed to v3-5-test.
Closing out bug report.