Fix upload authentication vulnerability only checked password, not us…

…ername This required to change the OPAN_AUTH_TOKENS, see the docs for detailsfix upload authentication vulnerability

only checked password, not username

This required to change the OPAN_AUTH_TOKENS, see the docs for details

Changes

@@ -1,6 +1,9 @@
 Revision history for App-opan
 
   - Fix upload argument name in docs
+  - Fix upload authentication vulnerability
+    only checked password, not username
+    This required to change the OPAN_AUTH_TOKENS, see the docs for details
 
 0.003002 - 2019-05-09
   - Document OPAN_MIRROR

script/opan

@@ -10,7 +10,7 @@ use Dist::Metadata;
 use File::Open qw(fopen);
 use List::UtilsBy qw(sort_by);
 use IPC::System::Simple qw(capture);
-use Mojo::Util qw(monkey_patch);
+use Mojo::Util qw(monkey_patch b64_encode);
 use Mojo::File qw(path);
 use Import::Into;
 
@@ -274,7 +274,8 @@ Mojo::IOLoop->recurring(600 => sub { do_pull(app); }) if $ENV{OPAN_RECURRING_PUL
 
 post "/upload" => sub {
   my $c = shift;
-  unless ($TOKENS{$c->req->url->to_abs->password || ""}) {
+  my $b64credentials = b64_encode($c->req->url->to_abs->userinfo || '', '');
+  unless ($TOKENS{$b64credentials}) {
     $c->res->headers->www_authenticate("Basic realm=opan");
     return $c->render(status => 401, text => "Token missing or not found\n");
   }
@@ -623,8 +624,10 @@ specified you wanted that).
 
 To enable the /upload endpoint, set the ENV var OPAN_AUTH_TOKENS to a colon
 separated list of accepted tokens for uploads. This will allow a post with a
-'dist' upload argument, checking http basic auth password against the provided
-auth tokens.
+'dist' upload argument, checking http basic auth credentials against the
+provided auth tokens.
+Each token is a base64 encoded username:password as sent in the Authorization:
+Basic http header.
 
 =head2 recurring pull
 

t/upload.t

@@ -7,7 +7,9 @@ use Mojo::File qw/tempdir path/;
 
 my $tempdir = tempdir(CLEANUP => 1);
 
-BEGIN { $ENV{OPAN_AUTH_TOKENS} = 'abc:bcd'; }
+# user1:pass
+# user2:foobar
+BEGIN { $ENV{OPAN_AUTH_TOKENS} = 'dXNlcjE6cGFzcw==:dXNlcjI6Zm9vYmFy'; }
 
 require "$FindBin::Bin/../script/opan";
 
@@ -16,16 +18,16 @@ my $t = Test::Mojo->new;
 $t->app->start('init');
 
 $t->post_ok('/upload')->status_is('401');
-$t->post_ok('/upload', {Authorization => "Basic OmFiYw=="})->status_is('400');
+$t->post_ok('/upload', {Authorization => "Basic dXNlcjE6cGFzcw=="})->status_is('400');
 my $upload = {dist => {content => 'HELLO', filename => 'world.tgz'}};
-$t->post_ok('/upload', {Authorization => "Basic OmFiYw=="}, form => $upload)->status_is('200');
+$t->post_ok('/upload', {Authorization => "Basic dXNlcjE6cGFzcw=="}, form => $upload)->status_is('200');
 my $f=$tempdir->child('test')->spurt('TEST');
 my $url=$t->ua->server->url->path('upload')->to_abs.'';
 
 if (eval { require CPAN::Uploader; 1 }) {
   Mojo::IOLoop->subprocess(sub {
           my $sub = shift;
-          return CPAN::Uploader->upload_file($f.'', {upload_uri => $url, user => 'foo', password => 'abc', debug => 1 });
+          return CPAN::Uploader->upload_file($f.'', {upload_uri => $url, user => 'user1', password => 'pass', debug => 1 });
       }, sub {
           my ($sub,$ret) = @_;
           ok(!$ret, 'no return is good return');