From 71fa5d09f7f0f71b6203bcb896ba4d55c9d43260 Mon Sep 17 00:00:00 2001 From: Christophe Maudoux Date: Fri, 26 Mar 2021 23:33:41 +0100 Subject: [PATCH] Be more strict with URL (#2477) --- .../NG/Portal/Plugins/CheckDevOps.pm | 22 ++++++------ .../Lemonldap/NG/Portal/Plugins/CheckUser.pm | 8 ++--- .../t/56-CheckDevOps-with-Download.t | 35 +++++++++++-------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckDevOps.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckDevOps.pm index 53eb01ac3..a8159f1d2 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckDevOps.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckDevOps.pm @@ -5,6 +5,7 @@ use Mouse; use JSON qw(from_json); use Lemonldap::NG::Common::UserAgent; use Lemonldap::NG::Portal::Main::Constants qw( + URIRE PE_OK PE_ERROR PE_BADURL @@ -124,27 +125,26 @@ sub run { # Check URL if allowed and exists if ( $self->conf->{checkDevOpsDownload} and $url = $req->param('url') ) { undef $url if $self->p->checkXSSAttack( 'CheckDevOps URL', $url ); - if ( $url && $url =~ m#^(?:https?://)?([^/]*)(.*)#i ) { + if ( $url && $url =~ URIRE ) { # Reformat url - my ( $vhost, $appuri ) = $url =~ m#^(?:https?://)?([^/]*)(.*)#i; - my ($proto) = $url =~ m#^(https?://).*#i; - $proto ||= 'http://'; - $url = "$proto$vhost/rules.json"; + my ( $proto, $vhost, $appuri ) = ( $2, $3, $5 ); + $url = "$proto://$vhost/rules.json"; my $resp = $self->ua->get( $url, 'Accept' => 'application/json' ); $self->logger->debug( "Code/Message from $url: " . $resp->code . '/' . $resp->message ); my $content = $resp->decoded_content; - $self->logger->debug("Content received from $url: $content") if $content; + $self->logger->debug("Content received from $url: $content") + if $content; if ( $resp->is_success ) { - $json = eval { from_json($content, { allow_nonref => 1 }) }; + $json = eval { from_json( $content, { allow_nonref => 1 } ) }; if ($@) { # Prepare form params undef $json; - $msg = 'PE' . PE_BAD_DEVOPS_FILE; + $msg = 'PE' . PE_BAD_DEVOPS_FILE; $self->userLogger->error( "CheckDevOps: bad 'rules.json' file retrieved from $url ($@)" ); @@ -153,7 +153,7 @@ sub run { else { # Prepare form params - $msg = 'PE' . PE_FILENOTFOUND; + $msg = 'PE' . PE_FILENOTFOUND; $self->userLogger->error( "CheckDevOps: Unable to download 'rules.json' file from $url" ); @@ -162,8 +162,8 @@ sub run { else { # Prepare form params - $msg = 'PE' . PE_BADURL; - $self->userLogger->error('CheckDevOps: bad provided URL'); + $msg = 'PE' . PE_BADURL; + $self->userLogger->error('CheckDevOps: bad URL provided'); } } unless ( $json || $msg ) { diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm index ad63914c4..b34d88ce5 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/CheckUser.pm @@ -192,7 +192,7 @@ sub check { }; return $self->p->sendJSONresponse( $req, $params ) if $req->wantJSON && $msg; - + # Display form return $self->p->sendHtml( $req, 'checkuser', params => $params ) if $msg; @@ -405,7 +405,7 @@ sub check { sub _resolveURL { my ( $self, $req, $url ) = @_; my ($proto) = $url =~ m#^(https?://).*#i; - my ( $vhost, $appuri ) = $url =~ m#^(?:https?://)?([^/]*)(.*)#i; + my ( $vhost, $appuri ) = $url =~ m@^(?:https?://)?([^/#]*)(.*)@i; my ($port) = $vhost =~ m#^.+(:\d+)$#; $port ||= ''; $vhost =~ s/:\d+$//; @@ -467,7 +467,7 @@ sub _userData { sub _authorization { my ( $self, $req, $uri, $attrs ) = @_; - my ( $vhost, $appuri ) = $uri =~ m#^https?://([^/]*)(.*)#; + my ( $vhost, $appuri ) = $uri =~ m@^https?://([^/#]*)(.*)@; my $exist = 0; $vhost =~ s/:\d+$//; @@ -489,7 +489,7 @@ sub _authorization { sub _headers { my ( $self, $req, $uri, $attrs, $savedUserData ) = @_; - my ($vhost) = $uri =~ m#^https?://([^/]*).*#; + my ($vhost) = $uri =~ m@^https?://([^/#]*).*@; $vhost =~ s/:\d+$//; $req->{env}->{HTTP_HOST} = $vhost; diff --git a/lemonldap-ng-portal/t/56-CheckDevOps-with-Download.t b/lemonldap-ng-portal/t/56-CheckDevOps-with-Download.t index 222499f49..fba55bf4e 100644 --- a/lemonldap-ng-portal/t/56-CheckDevOps-with-Download.t +++ b/lemonldap-ng-portal/t/56-CheckDevOps-with-Download.t @@ -143,9 +143,9 @@ ok( $res->{MSG} eq 'PE105', 'PE105' ) or print STDERR Dumper($res); count(4); -# Download file -# ------------- -$query = 'url=http://test3.example.com'; +# Bad URLs +# -------- +$query = 'url=test3.example.com'; ok( $res = $client->_post( '/checkdevops', @@ -157,19 +157,26 @@ ok( ); ok( $res = eval { from_json( $res->[2]->[0] ) }, 'Response is JSON' ) or print STDERR "$@\n" . Dumper($res); -ok( $res->{ALERTE} eq 'alert-info', 'alert-info found' ) +ok( $res->{MSG} eq 'PE37', 'Bad URL' ) or print STDERR Dumper($res); -ok( $res->{FILE} =~ /headers/, 'headers found' ) +count(3); + +# -------- +$query = 'url=http://test3.example.com#test'; +ok( + $res = $client->_post( + '/checkdevops', + IO::String->new($query), + cookie => "lemonldap=$id", + length => length($query), + ), + 'POST checkdevops with wrong url' +); +ok( $res = eval { from_json( $res->[2]->[0] ) }, 'Response is JSON' ) + or print STDERR "$@\n" . Dumper($res); +ok( $res->{URL} eq 'http://test3.example.com/rules.json', 'Well formated URL' ) or print STDERR Dumper($res); -ok( $res->{FILE} =~ /rules/, 'rules found' ) - or print STDERR Dumper($res); -ok( $res->{FILE} =~ /"\$uid ne qq#dwho#"/, 'rule found' ) - or print STDERR Dumper($res); -ok( $res->{URL} eq 'http://test3.example.com/rules.json', 'URL found' ) - or print STDERR Dumper($res); -ok( $res->{MSG} eq 'checkDevOps', 'MSG found' ) - or print STDERR Dumper($res); -count(8); +count(3); $client->logout($id); clean_sessions();