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

Re: code injection in packages.debian.org


Am 11. Dezember 2006 18:51 schrieb Javier Fernández-Sanguino Peña:
> On Mon, Dec 11, 2006 at 04:57:30PM +0100, Christian Boltz wrote:
     [please CC me in replies, I'm not subscribed]
> > it's easy to do some code injection in packages.debian.org:
> This is not code injection, it's cross site-scripting. Given that:

OK, maybe I used the wrong name - but cross site-scripting would imply 
that you need some scripting. In this case, it's a pure injection via a 
link, without any need for scripts ;-)

> - packages.debian.org does not have any kind of client authentication
> - packages.debian.org does not use SSL certificate
> this is as much a problem as somebody being able to setup a "fake"
> packages.debian.org or do MITM injection.

It's much easier to setup a link in a webpage than to setup a fake 
server, spread wrong DNS information etc.

> Not that I wouldn't want to see this fixed but, really, this is as
> low risk as it can get. Through XSS no one could retrieve user
> credentials and no one should be trusting (in this day an age) the
> information from a website that is not signed (through an SSL
> server-side certificatE).


The people download packages from there which are installed as root 
(pre-/postinstall scripts) and might contain harmful binaries.
Therefore a wrong md5sum is not "low risk"!

Oh, and SSL would not change anything here. One can still create a link
with a wrong md5sum to https://packages.debian.org, this time fully
trusted ;-)

> That being said. I've developed a fix for the download CGI
> application (attached). And will submit this as a bug.

--- download.pl 1 Dec 2006 08:42:27 -0000       1.27
+++ download.pl 11 Dec 2006 17:49:34 -0000
@@ -182,17 +182,28 @@
 $file = $input->param('file');
 param_error( "file" ) unless defined $file;
+# Make file fit in a regexp
+param_invalid ("file") if  $file !~ /^[\w\%\.\_\-]+$/;

BTW: Inside [...], you do _not_ need to escape special chars. Your
regex will allow backslashes in the filename - not necessarily what
you want.

And it allows other funny things like
(hmm, does this package's license meet your strict requirements? ;-)

 $md5sum = $input->param('md5sum');
 param_error( "md5sum" ) unless defined $md5sum;
+# Make md5sum fit in a regexp
+param_invalid ("md5sum") if  $md5sum !~ /^\w{32}$/;

Oh yes, very funny ;-)
(thanks to fefe again)

Again: It is not an option to check the md5sum with a regex, you have
to display the _correct_ md5sum. This means you should not pass it as
URL parameter, but read it from a file or database.

 $type = $input->param('type');
 param_error( "type" ) unless defined $type;
+# Make type fit in a regexp
+param_invalid ("type") if  $type !~ /^\w{1,10}$/;

The param name sounds like you should verify against a fixed list
(array) of allowed values.

 $arch = $input->param('arch');
 param_error( "arch" ) unless defined $arch;
+# Make arch fit in a regexp
+param_invalid ("arch") if  $arch !~ /^[\w\-]{1,10}$/;
+# And also check that it is in the list of supported archs
+param_invalid ("arch") if  ! defined ($arches{$arch});

arch is also something that should be checked against a fixed array.


Christian Boltz
"Never surf faster, than your guardian penguin can fly!"

Reply to: