[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#694519: marked as done (tpu: libcgi-pm-perl/3.59+dfsg-1+deb7u1 (pre-approval))

Your message dated Tue, 11 Dec 2012 21:41:58 +0000
with message-id <1355262118.4241.10.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#694519: tpu: libcgi-pm-perl/3.59+dfsg-1+deb7u1 (pre-approval)
has caused the Debian Bug report #694519,
regarding tpu: libcgi-pm-perl/3.59+dfsg-1+deb7u1 (pre-approval)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
694519: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=694519
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: tpu

Hi

On Sat, Nov 24, 2012 at 05:46:02PM +0100, intrigeri wrote:
> Hi,
> 
> Salvatore Bonaccorso wrote (24 Nov 2012 07:29:04 GMT) :
> > short addition to the mail before which I missed: For a possible t-p-u
> > upload I should choose 3.59+dfsg-1+deb7u1. Attached corrected debdiff.
> 
> TL;DR --> I recommend to accept this unblock request for t-p-u.
> 
> I have verified that I could reproduce the security issue on current
> Wheezy, that I could not reproduce it after applying this patch, and
> that the code still behaves well in the "good" situation (that is when
> $CRLF is followed by space) after applying this patch.
> 
> The patch looks sane, and I trust Salvatore has correctly
> cherry-picked it from upstream.
> 
> (BTW, in case someone wants to reproduce these results, one has to
> insert a "\r" in the example test case found on the initial report [1]
> for this security issue, else one cannot possibly check that the
> patched code still behaves well in the "good" situation; resulting
> testing code is:
> 
>   $ perl -Ilib -E 'use CGI qw/header/; print header( -cookie => [ "foo\r\nbar\r\nbaz", ],    -p3p    => [ "foo\r\nbar\r\nbaz", ],);'
> 
> and:
> 
>   $ perl -Ilib -E 'use CGI qw/header/; print header( -cookie => [ "foo\r\n bar\r\n baz", ],    -p3p    => [ "foo\r\n bar\r\n baz", ],);'
> )

Thanks for your review. To have this better tracked for the t-p-u part
I'm opening with this a bug against release.d.o.

@ReleaseTeam: This is about #693421 "CVE-2012-5526 CGI.pm: Newline
injection due to improper CRLF escaping in Set-Cookie and P3P
headers".

We could wait for some more testing in unstable for the version there.
The patch for tpu would be the "same" (the package cannot go trough
unstable -> testing).

Salvatore

diff -Nru libcgi-pm-perl-3.59+dfsg/debian/changelog libcgi-pm-perl-3.59+dfsg/debian/changelog
--- libcgi-pm-perl-3.59+dfsg/debian/changelog	2011-12-30 20:36:13.000000000 +0100
+++ libcgi-pm-perl-3.59+dfsg/debian/changelog	2012-11-24 08:27:32.000000000 +0100
@@ -1,3 +1,13 @@
+libcgi-pm-perl (3.59+dfsg-1+deb7u1) testing-proposed-updates; urgency=high
+
+  * Team upload.
+  * Add 0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch
+    [SECURITY] CVE-2012-5526: Newline injection due to improper CRLF
+    escaping in Set-Cookie and P3P headers.
+    Thanks to Niko Tyni <ntyni@debian.org> (Closes: #693421)
+
+ -- Salvatore Bonaccorso <carnil@debian.org>  Sat, 24 Nov 2012 07:39:11 +0100
+
 libcgi-pm-perl (3.59+dfsg-1) unstable; urgency=low
 
   * New upstream release
diff -Nru libcgi-pm-perl-3.59+dfsg/debian/gbp.conf libcgi-pm-perl-3.59+dfsg/debian/gbp.conf
--- libcgi-pm-perl-3.59+dfsg/debian/gbp.conf	1970-01-01 01:00:00.000000000 +0100
+++ libcgi-pm-perl-3.59+dfsg/debian/gbp.conf	2012-11-24 08:27:32.000000000 +0100
@@ -0,0 +1,2 @@
+[DEFAULT]
+debian-branch = wheezy
diff -Nru libcgi-pm-perl-3.59+dfsg/debian/patches/0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch libcgi-pm-perl-3.59+dfsg/debian/patches/0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch
--- libcgi-pm-perl-3.59+dfsg/debian/patches/0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch	1970-01-01 01:00:00.000000000 +0100
+++ libcgi-pm-perl-3.59+dfsg/debian/patches/0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch	2012-11-24 08:27:32.000000000 +0100
@@ -0,0 +1,67 @@
+From d5f9eaeea977edd24b3e6fdec7871ab254733ba4 Mon Sep 17 00:00:00 2001
+From: Ryo Anazawa <anazawa@cpan.org>
+Date: Wed, 14 Nov 2012 09:47:32 +0900
+Subject: [PATCH] CR escaping for P3P and Set-Cookie headers
+
+---
+ lib/CGI.pm  |   24 ++++++++++++------------
+ t/headers.t |    6 ++++++
+ 2 files changed, 18 insertions(+), 12 deletions(-)
+
+--- a/lib/CGI.pm
++++ b/lib/CGI.pm
+@@ -1501,8 +1501,17 @@
+                             'EXPIRES','NPH','CHARSET',
+                             'ATTACHMENT','P3P'],@p);
+ 
++    # Since $cookie and $p3p may be array references,
++    # we must stringify them before CR escaping is done.
++    my @cookie;
++    for (ref($cookie) eq 'ARRAY' ? @{$cookie} : $cookie) {
++        my $cs = UNIVERSAL::isa($_,'CGI::Cookie') ? $_->as_string : $_;
++        push(@cookie,$cs) if defined $cs and $cs ne '';
++    }
++    $p3p = join ' ',@$p3p if ref($p3p) eq 'ARRAY';
++
+     # CR escaping for values, per RFC 822
+-    for my $header ($type,$status,$cookie,$target,$expires,$nph,$charset,$attachment,$p3p,@other) {
++    for my $header ($type,$status,@cookie,$target,$expires,$nph,$charset,$attachment,$p3p,@other) {
+         if (defined $header) {
+             # From RFC 822:
+             # Unfolding  is  accomplished  by regarding   CRLF   immediately
+@@ -1546,18 +1555,9 @@
+ 
+     push(@header,"Status: $status") if $status;
+     push(@header,"Window-Target: $target") if $target;
+-    if ($p3p) {
+-       $p3p = join ' ',@$p3p if ref($p3p) eq 'ARRAY';
+-       push(@header,qq(P3P: policyref="/w3c/p3p.xml", CP="$p3p"));
+-    }
++    push(@header,"P3P: policyref=\"/w3c/p3p.xml\", CP=\"$p3p\"") if $p3p;
+     # push all the cookies -- there may be several
+-    if ($cookie) {
+-	my(@cookie) = ref($cookie) && ref($cookie) eq 'ARRAY' ? @{$cookie} : $cookie;
+-	for (@cookie) {
+-            my $cs = UNIVERSAL::isa($_,'CGI::Cookie') ? $_->as_string : $_;
+-	    push(@header,"Set-Cookie: $cs") if $cs ne '';
+-	}
+-    }
++    push(@header,map {"Set-Cookie: $_"} @cookie);
+     # if the user indicates an expiration time, then we need
+     # both an Expires and a Date header (so that the browser is
+     # uses OUR clock)
+--- a/t/headers.t
++++ b/t/headers.t
+@@ -22,6 +22,12 @@
+ like $cgi->header( -type => "text/html".$CGI::CRLF." evil: stuff " ),
+     qr#Content-Type: text/html evil: stuff#, 'known header, with leading and trailing whitespace on the continuation line';
+ 
++eval { $cgi->header( -p3p => ["foo".$CGI::CRLF."bar"] ) };
++like($@,qr/contains a newline/,'P3P header with CRLF embedded blows up');
++
++eval { $cgi->header( -cookie => ["foo".$CGI::CRLF."bar"] ) };
++like($@,qr/contains a newline/,'Set-Cookie header with CRLF embedded blows up');
++
+ eval { $cgi->header( -foobar => "text/html".$CGI::CRLF."evil: stuff" ) };
+ like($@,qr/contains a newline/,'unknown header with CRLF embedded blows up');
+ 
diff -Nru libcgi-pm-perl-3.59+dfsg/debian/patches/series libcgi-pm-perl-3.59+dfsg/debian/patches/series
--- libcgi-pm-perl-3.59+dfsg/debian/patches/series	2011-12-30 20:36:13.000000000 +0100
+++ libcgi-pm-perl-3.59+dfsg/debian/patches/series	2012-11-24 08:27:32.000000000 +0100
@@ -1 +1,2 @@
 man-cgi-fast.patch
+0001-CR-escaping-for-P3P-and-Set-Cookie-headers.patch

Attachment: signature.asc
Description: Digital signature


--- End Message ---
--- Begin Message --- 
On Tue, 2012-12-11 at 07:17 +0100, Salvatore Bonaccorso wrote:
> On Mon, Dec 10, 2012 at 09:37:17PM +0000, Adam D. Barratt wrote:
> > On Tue, 2012-11-27 at 08:27 +0100, Salvatore Bonaccorso wrote:
> > > @ReleaseTeam: This is about #693421 "CVE-2012-5526 CGI.pm: Newline
> > > injection due to improper CRLF escaping in Set-Cookie and P3P
> > > headers".
> > > 
> > > We could wait for some more testing in unstable for the version there.
> > > The patch for tpu would be the "same" (the package cannot go trough
> > > unstable -> testing).
> > 
> > fwiw, I've been having a look at the diff, and filtering out meta-data,
> > tests and documentation changes seems to give a reasonably sized diff:
[...]
> *But* if you agree on unblocking the version in unstable this is fine
> :-). I updated indeed in unstable 3.61 to 3.61-2 with the patch,
> instead of updating to 3.63 (new upstream release having the fix).

Having re-reviewed the diff, unblocked; thanks.

Regards,

Adam

--- End Message ---
Reply to: