It was reported that a stack buffer overflow vulnerability exists in multiple implementations of iSCSI target, including scsi-target-utils. A missing bounds check when handling SCN messages could result in a buffer overflow when the iSCSI Name string is longer than 1008 bytes, which could lead to corruption of sensitive stack data. During investigation, more buffer overflow vulnerabilities were discovered as well. Acknowledgements: Red Hat would like to thank the Vulnerability Research Team at TELUS Security Labs and Fujita Tomonori for responsibly reporting these flaws.
Adding Pavel to the cc, as the reporter.
It looks like this is going to have similar issues as CVE-2010-0743 where bad pdus can cause the target to crash.
Sorry, I've neglected this for a few weeks due to some other things that showed up. Mike, have you managed to reproduce this at all?
Added upstream scsi-target-utils to this issue. I had previously emailed him the embargoed details encrypted. Fujita, have you had a chance to look at what I sent you?
As Mike said, looks like a bad scn request can crash the target daemon. I think that we can fix this by checking the name length in print_scn_pdu(). The maximum length must be 223 bytes due to iSCSI spec. A bad request should be dropped.
Do you have a patch that corrects this? If so, please don't commit it to any public repository until we have a chance to alert the other iSCSI-related upstream projects that may also have the same problem and can coordinate a release date. However a patch to correct the issue that we can test, or get TELUS Security to test, would be welcome. Thanks!
I'll write a patch tomorrow and attach it here.
Created attachment 422756 [details] a fix for isns_attr_query() and send_scn_rsp() buffer overflow
Looks like isns_attr_query() in isns_handle() has the same problem. I've attached the patch to fix both (not tested).
Thank you, Fujita. Have you had an opportunity to test this since last week? I'm sorry that I've been unable to follow up on this. We've also assigned the name CVE-2010-2221 to this issue, so when it is time to make this public, please mention that CVE name in any commits or advisories regarding it.
I confirmed two bugs (isns_attr_query() and send_scn_rsp() buffer overflow) with the attached test tool. I also confirmed that the patch fixes both. However, I found a new problem in print_scn_pdu() that sending a bogus pdu can crash the target. Looks like qry_rsp_handle() has the same problem. Both assume that tlv->length is sane.
Created attachment 424334 [details] print_scn_pdu() and qry_rsp_handle() wrongly assume that tlv->length is sane. Sending invalid tlv->length pdu can crash the target. This patch fixes it.
So these last two issues are buffer overflows as well (i.e. all four bugs are buffer overflows)? If so, we don't need a new CVE. I'm going to try the testing tool today and try a build with the patches, in order to see if we can use this tool as a means of QA, verifying the fix.
Thanks for all of that information, Fujita. I'll see if I can get the reproducer working tonight or tomorrow. I think, since these are all buffer overflows, we can keep using the same CVE name (no need for further names since they will all be corrected at once). I will report back the results of the testing.
Ok, I've tested this and your patches look good, no crashes observed after the patches are applied. Thank you for the test program, it really helped.
Quick question for Pavel (from TELUS Security Labs): when you initially reported this to us, you mentioned that multiple products could be affected by this, not just scsi-target-utils. We also ship iscsi-initiator-utils and none of the affected functions exist, except qry_rsp_handle(). Have you tested whether or not iscsi-initiator-utils is affected by similar flaws?
Ok, I have confirmed with upstreams of IET and SCST that this does affect them as well. Unfortunately, despite the notification that this was embargoed and to not commit anything in public, SCST has the following new commit: http://scst.svn.sourceforge.net/viewvc/scst?view=revision&revision=1793 So an embargo at this point is largely academic and has to be quick. I'm waiting to hear back more from the IET folks, but if we can have our packages ready, at this point I think we should shoot for making this public on Wednesday or Thursday.
Also note that this does _NOT_ affect iscsi-initiator-utils as it does not have support for isns scns. As well, it has a bounds check when handling other packets, and when reading in the targetname in those other packets, it uses strlcpy.
(In reply to comment #11) > Created an attachment (id=422756) [details] > a fix for isns_attr_query() and send_scn_rsp() buffer overflow Instead of a static char wouldn't making sure there is a null terminator at vlen-1 or ISCSI_NAME_LEN accomplish the same? (char *) tlv->value + min(vlen - 1, ISCSI_NAME_LEN) = '\0' Then you can be rest assured it is null terminated AND of proper length. -Ross
> So an embargo at this point is largely academic and has to be quick. I'm > waiting to hear back more from the IET folks, but if we can have our packages > ready, at this point I think we should shoot for making this public on > Wednesday or Thursday. I'll merge the fixes tomorrow. Please let me know if you don't want me to do.
(In reply to comment #34) > (In reply to comment #11) > > Created an attachment (id=422756) [details] [details] > > a fix for isns_attr_query() and send_scn_rsp() buffer overflow > > Instead of a static char wouldn't making sure there is a null terminator at > vlen-1 or ISCSI_NAME_LEN accomplish the same? > > (char *) tlv->value + min(vlen - 1, ISCSI_NAME_LEN) = '\0' > > Then you can be rest assured it is null terminated AND of proper length. I guess that it works too. But I prefer not to modify the request buffer.
(In reply to comment #35) > > So an embargo at this point is largely academic and has to be quick. I'm > > waiting to hear back more from the IET folks, but if we can have our packages > > ready, at this point I think we should shoot for making this public on > > Wednesday or Thursday. > I'll merge the fixes tomorrow. Please let me know if you don't want me to do. I think Vlad let the cat out of the bag, but it's fine with me as we'll just take your patches. There was possible memory leak I found in going over the code, ini is allocated in qry_rsp_handle() but not free'd in free_all_acl(), but not related to the CVE, just FYI.
(In reply to comment #35) > > So an embargo at this point is largely academic and has to be quick. I'm > > waiting to hear back more from the IET folks, but if we can have our packages > > ready, at this point I think we should shoot for making this public on > > Wednesday or Thursday. > > I'll merge the fixes tomorrow. Please let me know if you don't want me to do. Can you hold off until Thursday? That gives me a chance to give the vendors a heads-up. I've not let them know about the issues since I wanted to make sure we had what projects were affected and patches available first. If we could set the embargo to July 1st at 14:00 UTC that would be fantastic.
I've just notified vendor-sec and noted the embargo date as being 20100701 14:00 UTC. We really do need to give more than 24hrs heads-up for this.
I don't know if anyone made any noise or commits on this yesterday (I didn't see anything), but I am going to be making this bug public in the next few hours; feel free to commit already if you don't want to wait. Also, if making commits or comments to this, please remember to include the CVE name in the commit message or advisory: CVE-2010-2221. Thanks!
This is fully public now, the TELUS Security Labs advisory: http://lists.grok.org.uk/pipermail/full-disclosure/2010-July/075478.html
The TELUS Security Labs report indicates there is the possibility of arbitrary code execution here, yet we've not heard that this is possible from the above comments. Is arbitrary code execution a possibility with any of these overflows?
Verified that the issue has been fixed in scsi-target-utils-0.0-6.20091205snap.el5_4.1 for both server and client based on comment#19 and attached test script (id=426449).
This issue has been addressed in following products: Red Hat Enterprise Linux 5 Via RHSA-2010:0518 https://rhn.redhat.com/errata/RHSA-2010-0518.html