[debian-security discussion list dropped, security team still in cc] On Tue, Jan 12, 2016 at 01:38:51PM +0000, Dominic Hargreaves wrote: > On Tue, Jan 12, 2016 at 12:54:19PM +0000, Chris Boot wrote: > > > Forwarded: https://rt.cpan.org/Public/Bug/Display.html?id=80346
> > > With Perl upgraded from 5.20.2-3+deb8u1 to 5.20.2-3+deb8u2, our > > > installation of TWiki (http://twiki.org/) no longer functions. This > > > happens due to CGI::Session::Driver::file complaining about taint. > I am happy to prepare an updated package with the patch in from the RT > ticket, though it would be good to get some second opinions on the > correctness of that patch. I guess that should be released as a DSA > update, given (as you point out) it's a regression indirectly introduced > by the DSA. Another alternative would be the jessie point release, which > for which the freeze date is later this week. The proposed fixes of untainting the session ID at the point where it's being used to generate a file name feel somewhat wrong to me. Shouldn't the data get untainted earlier when it gets read from the session file? I mostly agree with the reasoning in https://rt.cpan.org/Public/Bug/Display.html?id=80346#txn-1133036 about how the session file needs to be considered trusted: currently, depending on the serializer used, untainted data in a session can become tainted just because it got roundtripped in (presumably trusted) persistent storage. This suggests that the right place to untaint the data would be in the CGI::Session::Driver::*::retrieve() functions, or (more easily) centrally in CGI::Session::load(). Comments on the attached alternative patch? I'm a bit uneasy about throwing away $self->{_CLAIMED_ID} taintedness, but I expect that propagating it wouldn't fix the issue for real world applications, where the security is in attackers not being able to guess the session ID in the HTTP(s) cookies. (As a side note, I wonder a bit about hypothetical applications storing tainted data in a session variable, and getting it back untainted after the storage roundtrip. But I see that none of the current serializers preserve taintedness, and I suppose such applications could be declared broken.) -- Niko Tyni nt...@debian.org
>From bd47fa4892ac910b0f7fc0466d4e2699abdb6d94 Mon Sep 17 00:00:00 2001 From: Niko Tyni <nt...@debian.org> Date: Tue, 12 Jan 2016 23:40:53 +0200 Subject: [PATCH] Untaint raw data coming from session storage backends The various storage backends need to be considered trusted, so data coming out of them should be untainted. The _CLAIMED_ID comes from an HTTP cookie and is probably tainted, but presumably it's OK if it matched some data in the storage. Bug: https://rt.cpan.org/Public/Bug/Display.html?id=80346 Bug-Debian: https://bugs.debian.org/810799 --- lib/CGI/Session.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/CGI/Session.pm b/lib/CGI/Session.pm index 2788b04..6460d4d 100644 --- a/lib/CGI/Session.pm +++ b/lib/CGI/Session.pm @@ -724,6 +724,10 @@ sub load { # Requested session couldn't be retrieved return $self unless $raw_data; + # untaint; we trust the session backend, + # and presumably _CLAIMED_ID too at this point + $raw_data =~ /^(.*)$/s and $raw_data = $1; + my $serializer = $self->_serializer(); $self->{_DATA} = $serializer->thaw($raw_data); unless ( defined $self->{_DATA} ) { -- 2.6.4
