Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 593877 - (CVE-2010-2221) CVE-2010-2221 scsi-target-utils: stack buffer overflow vulnerability
CVE-2010-2221 scsi-target-utils: stack buffer overflow vulnerability
Status: CLOSED ERRATA
Product: Security Response
Classification: Other
Component: vulnerability (Show other bugs)
unspecified
All Linux
high Severity high
: ---
: ---
Assigned To: Red Hat Product Security
impact=important,public=20100701,repo...
: Security
Depends On: 604278 604279 604280
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-19 18:26 EDT by Vincent Danen
Modified: 2018-08-15 17:49 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-03-26 11:53:40 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
a fix for isns_attr_query() and send_scn_rsp() buffer overflow (1.63 KB, patch)
2010-06-09 23:02 EDT, fujita.tomonori
no flags Details | Diff
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. (1.20 KB, patch)
2010-06-15 22:58 EDT, fujita.tomonori
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2010:0518 normal SHIPPED_LIVE Important: scsi-target-utils security update 2010-07-08 11:07:47 EDT

  None (edit)
Description Vincent Danen 2010-05-19 18:26:15 EDT
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.
Comment 2 Vincent Danen 2010-05-19 18:32:00 EDT
Adding Pavel to the cc, as the reporter.
Comment 3 Mike Christie 2010-05-20 17:39:26 EDT
It looks like this is going to have similar issues as CVE-2010-0743 where bad pdus can cause the target to crash.
Comment 4 Vincent Danen 2010-06-04 13:14:44 EDT
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?
Comment 7 Vincent Danen 2010-06-08 23:55:57 EDT
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?
Comment 8 fujita.tomonori 2010-06-09 00:25:32 EDT
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.
Comment 9 Vincent Danen 2010-06-09 11:03:00 EDT
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!
Comment 10 fujita.tomonori 2010-06-09 11:16:12 EDT
I'll write a patch tomorrow and attach it here.
Comment 11 fujita.tomonori 2010-06-09 23:02:36 EDT
Created attachment 422756 [details]
a fix for isns_attr_query() and send_scn_rsp() buffer overflow
Comment 12 fujita.tomonori 2010-06-09 23:04:41 EDT
Looks like isns_attr_query() in isns_handle() has the same problem. I've attached the patch to fix both (not tested).
Comment 13 Vincent Danen 2010-06-15 14:59:37 EDT
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.
Comment 16 fujita.tomonori 2010-06-15 22:31:34 EDT
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.
Comment 17 fujita.tomonori 2010-06-15 22:58:56 EDT
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.
Comment 18 Vincent Danen 2010-06-21 16:34:14 EDT
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.
Comment 21 Vincent Danen 2010-06-24 18:22:06 EDT
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.
Comment 22 Vincent Danen 2010-06-25 18:14:49 EDT
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.
Comment 24 Vincent Danen 2010-06-25 18:45:10 EDT
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?
Comment 30 Vincent Danen 2010-06-28 16:03:18 EDT
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.
Comment 31 Vincent Danen 2010-06-28 16:04:50 EDT
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.
Comment 34 Ross Walker 2010-06-28 19:08:23 EDT
(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
Comment 35 fujita.tomonori 2010-06-28 23:14:05 EDT
> 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.
Comment 36 fujita.tomonori 2010-06-28 23:22:00 EDT
(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.
Comment 37 Ross Walker 2010-06-28 23:24:54 EDT
(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.
Comment 39 Vincent Danen 2010-06-29 12:05:22 EDT
(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.
Comment 40 Vincent Danen 2010-06-29 12:28:56 EDT
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.
Comment 47 Vincent Danen 2010-07-02 12:37:18 EDT
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!
Comment 48 Vincent Danen 2010-07-03 00:57:59 EDT
This is fully public now, the TELUS Security Labs advisory:

http://lists.grok.org.uk/pipermail/full-disclosure/2010-July/075478.html
Comment 49 Vincent Danen 2010-07-05 19:46:36 EDT
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?
Comment 50 michal novacek 2010-07-07 06:33:41 EDT
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).
Comment 51 errata-xmlrpc 2010-07-08 11:07:50 EDT
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

Note You need to log in before you can comment on or make changes to this bug.