Minimal fixes for open redirect flaw.

sympa-community · Sep 7, 2018 · c6ce32a · c6ce32a
1 parent f208a7b
commit c6ce32a
Showing 1 changed file with 21 additions and 8 deletions.
diff --git a/src/cgi/wwsympa.fcgi.in b/src/cgi/wwsympa.fcgi.in
@@ -3160,9 +3160,9 @@ sub do_login {
     my $user;
     my $next_action;
 
-    if ($in{'referer'}) {
-        $param->{'redirect_to'} =
-            Sympa::Tools::Text::unescape_chars($in{'referer'});
+    my $url_redirect;
+    if ($url_redirect = _clean_referer($in{'referer'})) {
+        $param->{'redirect_to'} = $url_redirect;
     } elsif ($in{'previous_action'}
         && $in{'previous_action'} !~ /^(login|logout|loginrequest)$/) {
         $next_action = $in{'previous_action'};
@@ -3219,8 +3219,8 @@ sub do_login {
         if ($url_redirect = is_ldap_user($in{'email'})) {
             $param->{'redirect_to'} = $url_redirect
                 if $url_redirect ne 'none';
-        } elsif ($in{'failure_referer'}) {
-            $param->{'redirect_to'} = $in{'failure_referer'};
+        } elsif ($url_redirect = _clean_referer($in{'failure_referer'})) {
+            $param->{'redirect_to'} = $url_redirect;
         } else {
             $in{'init_email'} = $in{'email'};
             $param->{'init_email'} = $in{'email'};
@@ -3276,12 +3276,14 @@ sub do_login {
         } else {
             $param->{'login_error'} = 'wrong_password';
         }
+
+        my $url_redirect;
         if ($in{'previous_action'}) {
             delete $in{'passwd'};
             $in{'list'} = $in{'previous_list'};
             return $in{'previous_action'};
-        } elsif ($in{'failure_referer'}) {
-            $param->{'redirect_to'} = $in{'failure_referer'};
+        } elsif ($url_redirect = _clean_referer($in{'failure_referer'})) {
+            $param->{'redirect_to'} = $url_redirect;
         } else {
             return 'renewpasswd';
         }
@@ -3410,6 +3412,15 @@ sub do_login {
     return 1;
 }
 
+sub _clean_referer {
+    my $referer = shift;
+
+    return undef
+        unless $referer and $referer =~ m{\Ahttps?://}i;
+
+    return $referer;
+}
+
 ## Login WWSympa
 ## The sso_login action is made of 4 subactions that make a complete workflow.
 ## Note that this comlexe workflow is only used if the SSO server does not
@@ -11631,7 +11642,9 @@ sub do_d_read {
     # File or directory?
 
     if ($shared_doc->{type} eq 'url') {
-        $param->{'redirect_to'} = $shared_doc->{url};
+        $param->{'redirect_to'} = $shared_doc->{url}
+            if $shared_doc->{url}
+            and $shared_doc->{url} =~ m{\Ahttps?://}i;
         return 1;
     } elsif ($shared_doc->{type} eq 'file') {
         $param->{'content_type'} = $shared_doc->{mime_type};