[aur-dev] [PATCH] Redirect back after login

[canyonknight]

On Fri, Dec 14, 2012 at 5:51 PM, Marcel Korpel <marcel.lists at gmail.com> wrote:
> Fixes FS#32481
> ---
>  web/html/login.php        |  2 +-
>  web/lib/acctfuncs.inc.php |  8 +++++++-
>  web/lib/aur.inc.php       | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  web/template/header.php   |  4 ++--
>  4 files changed, 54 insertions(+), 4 deletions(-)

This implementation is susceptible to HTTP header injection. Also note
the usage of $_SERVER['REQUEST_URI'] had previously been eliminated
with commit 630f1cbae8473fb05e5f5af7244eccc60fe93812.

>
> diff --git a/web/html/login.php b/web/html/login.php
> index e458fec..48fda29 100644
> --- a/web/html/login.php
> +++ b/web/html/login.php
> @@ -20,7 +20,7 @@ html_header('AUR ' . __("Login"));
>                 <a href="<?= get_uri('/logout/'); ?>">[<?= __("Logout"); ?>]</a>
>         </p>
>         <?php elseif (!$DISABLE_HTTP_LOGIN || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'])): ?>
> -       <form method="post" action="<?= get_uri('/login') ?>">
> +       <form method="post" action="<?= get_uri('/login') . redirect_post() ?>">
>                 <fieldset>
>                         <legend><?= __('Enter login credentials') ?></legend>
>                         <?php if (!empty($login_error)): ?>
> diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php
> index 3fd23ae..ee19511 100644
> --- a/web/lib/acctfuncs.inc.php
> +++ b/web/lib/acctfuncs.inc.php
> @@ -460,7 +460,13 @@ function try_login($dbh=NULL) {
>                                         $cookie_time = 0;
>
>                                 setcookie("AURSID", $new_sid, $cookie_time, "/", null, !empty($_SERVER['HTTPS']), true);
> -                               header("Location: " . get_uri('/'));
> +
> +                               if (isset($_GET['redirect'])) {
> +                                       header("Location: " . $_GET['redirect']);
> +                               } else {
> +                                       header("Location: " . get_uri('/'));
> +                               }
> +
>                                 $login_error = "";
>
>                         }
> diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php
> index 018d5c8..653cf55 100644
> --- a/web/lib/aur.inc.php
> +++ b/web/lib/aur.inc.php
> @@ -310,6 +310,50 @@ function html_header($title="") {
>  }
>
>  /**
> + * Add redirect URL parameter when appropriate
> + * @return string Query string
> + **/
> +function redirect_string() {
> +       global $USE_VIRTUAL_URLS;
> +
> +       /* get the request URI without the optional query string */
> +       $uri_parts = explode('?', $_SERVER['REQUEST_URI']);
> +
> +       /* remove leading slash if get_route() is used */
> +       if (!$USE_VIRTUAL_URLS) {
> +               $uri_parts[0] = ltrim($uri_parts[0], '/');
> +       }
> +
> +       /* don't add a redirect string to /login/ to prevent looping;
> +          don't add a redirect string to / and /register/: it's useless */
> +       switch ($uri_parts[0]) {
> +               case get_uri('/'):
> +               case get_uri('/login/'):
> +               case get_uri('/register/'):
> +                       $querystring = '';
> +                       break;
> +               default:
> +                       $querystring = '?redirect=' . urlencode($_SERVER["REQUEST_URI"]);
> +       }
> +
> +       return htmlentities($querystring);
> +}
> +
> +/**
> + * Add redirect URL parameter to form action
> + * @return string Query string
> + **/
> +function redirect_post() {
> +       if ( isset($_GET['redirect']) ) {
> +               $querystring = '?redirect=' . urlencode($_GET['redirect']);
> +       } else {
> +               $querystring = '';
> +       }
> +
> +       return htmlentities($querystring);
> +}
> +
> +/**
>   * Common AUR footer displayed on all pages
>   *
>   * @param string $ver The AUR version
> diff --git a/web/template/header.php b/web/template/header.php
> index 92cb2ff..e073df5 100644
> --- a/web/template/header.php
> +++ b/web/template/header.php
> @@ -64,9 +64,9 @@
>                                         <?php else: ?>
>                                                 <li><a href="<?= get_uri('/register/'); ?>"><?= __("Register"); ?></a></li>
>                                                 <?php if ($DISABLE_HTTP_LOGIN && empty($_SERVER['HTTPS'])): ?>
> -                                               <li><a href="<?= $AUR_LOCATION . get_uri('/login/'); ?>"><?= __("Login"); ?></a></li>
> +                                               <li><a href="<?= $AUR_LOCATION . get_uri('/login/') . redirect_string(); ?>"><?= __("Login"); ?></a></li>
>                                                 <?php else: ?>
> -                                               <li><a href="<?= get_uri('/login/'); ?>"><?= __("Login"); ?></a></li>
> +                                               <li><a href="<?= get_uri('/login/') . redirect_string(); ?>"><?= __("Login"); ?></a></li>
>                                                 <?php endif; ?>
>                                         <?php endif; ?>
>                                 </ul>
> --
> 1.8.0.2
>