View Issue Details

IDProjectCategoryView StatusLast Update
0026399mantisbtsecuritypublic2019-12-09 15:04
Reporteranfrind Assigned Todregad  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionwon't fix 
Product Version2.20.0 
Summary0026399: Username shown in URL if password rejected
Description

If a user tries and fails to log in, the username they entered will appear as part of the URL's HTTP GET parameters, e.g.:

https://mantisbt.org/bugs/login_page.php?error=1&username=fakeaccount&return=index.php&secure_session=1

This could result in usernames being inadvertently saved to server and/or proxy logs, or leaked in various other ways.

Steps To Reproduce
  1. If necessary, log out of Mantis.
  2. Enter a username (it doesn't matter if it's valid or not).
  3. Enter an incorrect password.
  4. Observe that when you are brought back to the first login page, the username you entered is in the URL.
TagsNo tags attached.

Activities

dregad

dregad

2019-11-27 03:17

developer   ~0063149

Considering that usernames in MantisBT are pretty much public information as they are displayed all over the place in various screens, I do not think this that displaying it in a failed login URL is causing extra exposure.

anfrind

anfrind

2019-11-27 12:46

reporter   ~0063150

On my team's Mantis server, we disallow anonymous access, so usernames should only be visible to authenticated users.

dregad

dregad

2019-11-28 13:19

developer   ~0063158

The username is provided in the URL as a convenience to the user, so they do not have to type it again after a failed login.

Unfortunately, this has to be done with a GET request, because it is issued by a redirect from login.php. We can't do POST from a redirect without resorting to ugly hacks e.g. using javascript, and we're not going to do that.

The only option I can think of to satisfy your requirement, would be to only add username=xxx to the query, if anonymous login is enabled (or to never add it at all).

diff --git a/login.php b/login.php
index 29df67289..3fd7d487b 100644
--- a/login.php
+++ b/login.php
@@ -74,10 +74,14 @@ if( auth_attempt_login( $f_username, $f_password, $f_perm_login ) ) {
 } else {
    $t_query_args = array(
        'error' => 1,
-       'username' => $f_username,
        'return' => $t_return,
    );

+   # Do not expose the username if anonymous access is disabled
+   if( auth_anonymous_enabled() ) {
+       $t_query_args['username'] = $f_username;
+   }
+
    if( $f_reauthenticate ) {
        $t_query_args['reauthenticate'] = 1;
    }

Quite frankly I see the risk of such exposure as negligible and I don't really think it's worth implementing that in core. You're welcome to customize your instance as indicated above.

Issue History

Date Modified Username Field Change
2019-11-26 21:39 anfrind New Issue
2019-11-27 03:17 dregad Note Added: 0063149
2019-11-27 12:46 anfrind Note Added: 0063150
2019-11-28 13:19 dregad Assigned To => dregad
2019-11-28 13:19 dregad Status new => resolved
2019-11-28 13:19 dregad Resolution open => won't fix
2019-11-28 13:19 dregad Note Added: 0063158
2019-12-09 15:04 atrol Status resolved => closed