|
Bugzilla – Full Text Bug Listing |
|
Description
Thomas Biege
2004-06-14 18:14:14 UTC
<!-- SBZ_reproduce --> - Created attachment 21118 [details]
ISC Advisory
Created attachment 21119 [details]
includes patches
Created attachment 21120 [details]
another vulnerability with using vsprintf instead of vsnprintf on some platforms
CDR: Monday June 14. 2pm EST (means 21:00 local time if I'm not wrong) Okay. Can we expect further patches for the vsnprintf / VU#654390 issue before then? JFYI: since SLES8, our dhcpd is run chrooted and as non-root user by default. Therefore the impact might be lower for us than for others. I don't think anyone else is using the chroot patch, but I don't know for sure. Maybe we use vsnprintf instead of vsprintf Created attachment 21122 [details]
patchinfo-box.dhcp
Created attachment 21123 [details]
patchinfo.dhcp
Created attachment 21124 [details]
patchinfo-box.dhcp-server
Created attachment 21126 [details]
patchinfo.dhcp-server
Unfotunately, on Linux: #define vsnprintf(buf, size, fmt, list) vsprintf (buf, fmt, list) #define NO_SNPRINTF It is mosly enough if we remove that stuff.
And_ add -DHAVE_SNPRINTF, because in two places I see:
#if defined (HAVE_SNPRINTF)
/* Presumably if we have snprintf, we also have
vsnprintf. */
vsnprintf (tbuf, sizeof tbuf, fmt, va);
#else
vsprintf (tbuf, fmt, va);
#endif
(Although the configure script does not check for snprintf and does never
define HAVE_SNPRINTF.)
The sprintf() calls in ./server/dhcp.c +278 and onwards are already
fixed by the other patch where the client hostname is limited to 64
characters.
Created attachment 21160 [details]
patch to use vsnprintf() instead of vsprintf() calls
Ok, thanks for your effort! Please also create a version for /work/SRC/old-versions/8.1/SLES/arch/s390/dhcp Yes, will do that new CDR: Thursday June 16 at 2pm EST s/CDR/CRD/ CAN-2004-0460 VU#317350 ISC DHCPD contains a stack buffer overflow vulnerability in handling log lines containing ASCII characters only CAN-2004-0461 VU#654390 ISC DHCP contains C includes that define "vsnprintf" to "vsprintf" creating potential buffer overflow conditions *grmpf* the CRD was wrong. CRD: Thursday *June 17* at 2pm EST Created attachment 21196 [details]
more information about vnsprintf bug
All packages have been fixed and submitted now: /work/SRC/old-versions/7.2/all/dhcp -> /work/src/done/SLES7 /work/SRC/old-versions/7.2/arch/s390/dhcp -> /work/src/done/SLES7/7.2-s390 /work/SRC/old-versions/7.2/arch/sles-i386/dhcp -> /work/src/done/SLES7/sles-i386 /work/SRC/old-versions/7.2/arch/sles-s390x/dhcp -> /work/src/done/SLES7-S390X /work/SRC/old-versions/7.3/all/dhcp -> /work/src/done/SLES7-PPC /work/SRC/old-versions/8.0/all/dhcp -> /work/src/done/8.0 /work/SRC/old-versions/8.1/UL/all/dhcp -> /work/src/done/8.1 /work/SRC/old-versions/8.1/SLES/arch/s390/dhcp -> /work/src/done/SLES8/s390 /work/SRC/old-versions/8.2/all/dhcp -> /work/src/done/8.2 /work/SRC/old-versions/9.0/all/dhcp -> /work/src/done/9.0 /work/SRC/old-versions/9.1/SLES/all/dhcp -> /work/src/done/9.1 STABLE is also already fixed. I have also submitted the above 4 patchinfo files now. Reassigning to Thomas for further processing :) Thanks! Please wait a minute. I'm checking the PACKAGE lines in the patchinfos... The /mounts/work/src/done/PATCHINFO/patchinfo-box.dhcp was not needed (the one from 'edit_patchinfo -b dhcp') What's with patchinfo.dhcp? The "minres" library? Created attachment 21254 [details]
dhcp-3.0.1.tar.gz
Below is an encrypted gzipped tarball containing patch information.
If you are unable to extract this file, please let us know.
Thank you,
Jason Rafail
==============================
Member of the Technical Staff
CERT Coordination Center
Software Engineering Institute
Carnegie Mellon University
4500 Fifth Avenue
Pittsburgh, PA 15213
1-412-268-7090
==============================
CRD: Tuesday, June 22 @ 2pm Created attachment 21301 [details]
and another CERT mail
Created attachment 21307 [details]
new patchinfo.dhcp superseding attachmeht 13123, which contained a wrong description by mistake
...which answers Michaels question. (The mentioned minires vulnerability was a previous security update) Created attachment 21354 [details]
2004-06-18_1.txt
seems like only rc12 and rc13 are exploitable. nevertheless better we fixed it
completely.
ISC has confirmed that the release date of June 22 at 2pm EST. Please update any documentation that you plan to release with this date. cross-references: VU#317350 (buffer overflow) VU#654390 (using vsprintf()) Two vendors have done code analysis and confirmed that the flawed code from VU#654390 is in the DHCP 2.x releases. It is not believed that the flawed code from VU#317350 is present in DHCP 2.x. We are currently doing some code analysis of DHCP 2.0 to deterimine if VU#654390 is exploitable in this DHCP 2.0. One vendor does not believe that VU#654390 is exploitable in these versions. Some of our older packages (SLES7) also ship the 2.0 version in addition to 3.x. 3.x was used by default; 2.0 could be used only by editing the init script. I have not looked at 2.0. Can you have a look an it please? I'm currently looking at it Created attachment 21425 [details]
Concise diff between 3.0.1rc13 and proposed 3.0.1 tarball
I stripped the voluminous license/comment changes, and all parts not
relevant on Linux.
I checked all sprintf calls in the dhcp-2.0pl5 sources that are built for backwards compatibility in our 7.2/7.3 based packages. None of them is relevant for us. (I'm deliberately ignoring the logging of warnings during parsing of the configuration, since no client supplied data is involved there.) Great! Thanks for the good news. Regarding ISC's proposed tarball, I went through the diff and saw that they have additionally added some code to server/ddns.c (unmentioned so far...) to restrict the length of client provided fqdn and calculated reverse domain name to 255 characters altogether. I'm trying to find out which are the security relevant effects of this... I don't understand why they do it because I fails to see an influence on the vulnerabilities that they talked about. In ddns_update() and minires_mkupdrec() called from there (ddns.c), the names are stored in variable length buffers, afaics. Could someone ask the ISC folks about their reasoning of this change, or can I contact them directly? If the change is relevant, we'd need another patch round... --- dhcp-3.0.1rc13/server/ddns.c 2002-11-17 03:29:30.000000000 +0100 +++ dhcp-3.0.1/server/ddns.c 2004-06-14 23:08:50.000000000 +0200 @@ -345,6 +336,12 @@ &lease -> scope, oc, MDL); if (s1 && s2) { + if (ddns_hostname.len + ddns_domainname.len > 253) { + log_error ("ddns_update: host.domain name too long"); + + goto out; + } + buffer_allocate (&ddns_fwd_name.buffer, ddns_hostname.len + ddns_domainname.len + 2, MDL); @@ -449,6 +446,11 @@ if (!ddns_fwd_name.len) goto out; + if (ddns_fwd_name.len > 255) { + log_error ("client provided fqdn: too long"); + goto out; + } + /* * Compute the RR TTL. */ @@ -479,7 +481,13 @@ packet -> options, state -> options, &lease -> scope, oc, MDL); - + + if (d1.len > 238) { + log_error ("ddns_update: Calculated rev domain name too long."); + s1 = 0; + data_string_forget (&d1, MDL); + } + if (oc && s1) { /* Buffer length: XXX.XXX.XXX.XXX.<ddns-rev-domain-name>\0 */
[Sorry. Let's try again.]
Regarding ISC's proposed tarball, I went through the diff and saw that
they have additionally added some code to server/ddns.c (unmentioned so
far...) to restrict the length of client provided fqdn and calculated
reverse domain name to 255 characters altogether.
I'm trying to find out which are the security relevant effects of
this... I don't understand why they do it because I fails to see an
influence on the vulnerabilities that they talked about. In
ddns_update() and minires_mkupdrec() called from there (ddns.c), the
names are stored in variable length buffers, afaics.
Could someone ask the ISC folks about their reasoning of this change, or
can I contact them directly?
If the change is relevant, we'd need another patch round...
--- dhcp-3.0.1rc13/server/ddns.c 2002-11-17 03:29:30.000000000 +0100
+++ dhcp-3.0.1/server/ddns.c 2004-06-14 23:08:50.000000000 +0200
@@ -345,6 +336,12 @@
&lease -> scope, oc, MDL);
if (s1 && s2) {
+ if (ddns_hostname.len + ddns_domainname.len > 253) {
+ log_error ("ddns_update: host.domain name too long");
+
+ goto out;
+ }
+
buffer_allocate (&ddns_fwd_name.buffer,
ddns_hostname.len + ddns_domainname.len + 2,
MDL);
@@ -449,6 +446,11 @@
if (!ddns_fwd_name.len)
goto out;
+ if (ddns_fwd_name.len > 255) {
+ log_error ("client provided fqdn: too long");
+ goto out;
+ }
+
/*
* Compute the RR TTL.
*/
@@ -479,7 +481,13 @@
packet -> options,
state -> options,
&lease -> scope, oc, MDL);
-
+
+ if (d1.len > 238) {
+ log_error ("ddns_update: Calculated rev domain name too long.");
+ s1 = 0;
+ data_string_forget (&d1, MDL);
+ }
+
if (oc && s1) {
/* Buffer length:
XXX.XXX.XXX.XXX.<ddns-rev-domain-name>\0 */
The only other restriction that I see is this one: ./includes/arpa/nameser.h:#define NS_MAXDNAME 1025 /* maximum domain name */ It is enforced when the DDNS update package constructed (res_nmkupdate() in minires/res_mkupdate.c). The above would prevent DDNS updates with truncated domain names. I don't see the security implications here. A statement from ISC would be most helpful. Nice spotting. If you want to contact them: Here is the pgp key provided for mail to dhcp@isc.org: -----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v1.2.3 (FreeBSD) mQGiBEDPh3ERBACxrGSQjWzRJ4xypJzABwsUYNk7P0tv2Zy1Hwa5uOfxkZA8z/L5 FyR0L7/jFxJEpjeW1VvpEp8EJanX8oHU3kwwAVhTQw8WgcSrv/WZcLh7AvDYXABX AVcouWOpOrBrq8a/ro7fGbV+K60bD40jNB6YczL3bmcZVmxRc09kmUSmfwCgoMBU WVXHTN0OW47UpOKdtXcPZX0D/ig5mGeWXB3mKUqgdZb79aVTEDuZmZGy8wRQ4IQx tBYjM10vhaSlqxHSFUWQLRq5EKSLicNAyB2gg0qf3fCgpWYIYYdfKyHfNjx69MW9 t13Dzoz7hqDJJpo/fLGHJeY33GnDpGR6euB/c0QH/IhxwAbEkBA0YuIzjt++koYR xGudA/98bQFLgfaXWmK5ZVJG13WzkUltFQKOqx7S4OIlQXMrGiM20sxSaIlY1Kx4 G67AFJKaGsnSU3k/pJtonQoiex/yae8gyXlpet1rTfnWGrYar/lb8Ih9PiQVmp4u 10Ufdcvmc9laRq4EP+Cs2BJ15nGBC5tHpnjAeXOPCr1YcHsEMrQeSVNDIERIQ1Ag KHJvbGUpIDxkaGNwQGlzYy5vcmc+iGUEExECACUFAkDPh3ECGwMFCQlmAYAHCwkI BwMCAQMVAgMDFgIBAh4BAheAAAoJEPQaoFE8wTKcYq0AniBh35eLWlQ9AsgXejGu HpqUaW6mAJwLqKfN9X9+IG8weeyNzhelruVtKLkBDQRAz4dyEAQArETDaWrsWfiU EDCKPAmmPnuZq0W6Znus9po+mRWERvUfhJWIg0+jl6BqZ68Vh7NeYlZanFJtwt/k JxWf63OuqxkJApQJgjeBAEdVMQxZ7Sti7MNF0ewJu8NRmwTHNRzh7EvQdb9hOaaH L3aKI8e8R1W1OPnPv2TgJG1Etf8ApS8AAwUD/1er1xeGcGi6HarA8k6vnJsfZ7qB Pbu+NSFLLL0DO+5p0kdVOIj0VcqEjFZmDgsQIWsbt4VcpICd6bGUVt7eHYGqma0U HJc51uquVQEwWKEZyZWFoEriISJdD9kz9wWOpCmLJ0v8GBX0qiiczxhRaUMxmXDN 8dmisyXTnRIXkBTsiE8EGBECAA8FAkDPh3ICGwwFCQlmAYAACgkQ9BqgUTzBMpyx WgCfcs9ok9Y/VNGraBm92FvxdYJ8zaYAn1VAPZXRC8h3jG7pYHrbErYiX2mJ =oCH7 -----END PGP PUBLIC KEY BLOCK----- If you dislike talking with them let me know... Good, I asked them about the change. On Tue, Jun 22, 2004 at 03:46:34PM +0000, Paul Vixie wrote:
> that other change came about as a result of our format string audit.
> we strongly recommend that you apply it, even if it means recutting
> your packages.
What has truncating packets to do with format strings? packages approved. therefore i'll close this bug. Peter, should we move the packet-truncating issue to a new bug? No idea what they meant. Without signing an NDA they don't seem to be willing to disclose more details. (Did anyone of us ever sign an NDA with them, on behalf of SUSE?) Regarding a new bugzilla entry, I'd prefer tracking it here, because this bug already contains all required information. I think nobody signed an NDA with them... <!-- SBZ_reopen -->Reopened by poeml@suse.de at Wed Jun 23 15:02:14 2004, took initial reporter thomas@suse.de to cc I'm reopening, setting severity to normal, and then I'm going to add the missing hunk to our packages. I have submitted a new set of packages with the fix to server/ddns.c added (see comment #43): /work/SRC/old-versions/7.2/all/dhcp -> /work/src/done/SLES7 /work/SRC/old-versions/7.2/arch/s390/dhcp -> /work/src/done/SLES7/7.2-s390 /work/SRC/old-versions/7.2/arch/sles-i386/dhcp -> /work/src/done/SLES7/sles-i386 /work/SRC/old-versions/7.2/arch/sles-s390x/dhcp -> /work/src/done/SLES7-S390X /work/SRC/old-versions/7.3/all/dhcp -> /work/src/done/SLES7-PPC /work/SRC/old-versions/8.0/all/dhcp -> /work/src/done/8.0 /work/SRC/old-versions/8.1/SLES/arch/s390/dhcp -> /work/src/done/SLES8/s390 /work/SRC/old-versions/8.1/UL/all/dhcp -> /work/src/done/8.1 /work/SRC/old-versions/8.2/all/dhcp -> /work/src/done/8.2 /work/SRC/old-versions/9.0/all/dhcp -> /work/src/done/9.0 /work/SRC/old-versions/9.1/SLES/all/dhcp -> /work/src/done/9.1 Do you need a patchinfo file? Created attachment 21634 [details]
patchinfo (ddns fix) for SLES (dhcp-server)
Created attachment 21635 [details]
patchinfo (ddns fix) for SLES (dhcp)
Created attachment 21636 [details]
patchinfo (ddns fix) for box
I have just submitted those. Waiting for approval by security since Monday :-( approved CVE-2004-0461: CVSS v2 Base Score: 10.0 (AV:N/AC:L/Au:N/C:C/I:C/A:C) |