DEV Community

loading...
Cover image for SQL: Where spaces may not matter

SQL: Where spaces may not matter

antogarand profile image Antony Garand ・4 min read

Introduction

Here is the source to a voluntarily vulnerable application:

https://gitlab.com/AntonyGarand/pwn_fix_repeat_size_does_matter

It will soon be added as a challenge on the pwnfixrepe.at website, where you can work on fixing such security issues.

Your mission, if you accept it, is to login as administrator on the application.

If you wish to skip to the details, head to the solution section.


Application

The application itself is quite simple.

There are three features: Log in, log out and register.

Here is what the table look likes: (Source)

CREATE TABLE `users` (
  `id` int(11) PRIMARY KEY,
  `username` varchar(20) NOT NULL,
  `password` varchar(255) NOT NULL,
  `description` varchar(255) NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

INSERT INTO `users` (`id`, `username`, `password`, `description`) VALUES (1, 'admin', '$up3rS3cr3tP4$$w0rd!', 'You are admin!');

All of those features are using prepared statements and are not vulnerable to SQL injection, such as with the following login function: (Source)

function login() {
    global $db;
    if ( /* user or pass empty or not string */ ) {
        return false;
    }
    $query = $db->prepare('SELECT * FROM users WHERE username = :username and password = :password');
    $query->bindParam(':username', $_POST['user']);
    $query->bindParam(':password', $_POST['pass']);
    $query->execute();
    $result = $query->fetch();
    if ($result) {
        $_SESSION['username'] = $result['username'];
        echo 'Logged in succesfully!';
    } else {
        echo 'Invalid credentials!';
    }
}

Finally, if the current user is logged in, we show a greeting message:

/* if current user is logged in */ 
{ ?>
    <div>
        Welcome back, <?= htmlspecialchars($currentUser['username']) ?>!<br/>
        <?= $currentUser['description'] ?>
    </div>
    <!-- ... -->
    <?php
}

Analysis

The vulnerability is not within PHP, but rather is caused by SQL itself, and many bad design decisions with the vulnerable application.

Here are few observations on the users table:

  • The username is not unique
  • The username is a varchar of length 20

And on the application itself:

  • The logged in user is based off the username
// If login success: 
$_SESSION['username'] = $result['username'];
  • You can only register a user if the username isn't already taken
if (selectUser($_POST['user'])) {
    echo 'Username already taken!';
    return false;
}

Solution

Comparing strings in SQL is interesting.

What would you expect the following statement to be?

SELECT * FROM (select 1) as t WHERE 'x' = 'x '

Unlike what many would expect, the 'x' = 'x ' condition is true!

In SQL, when comparing two strings, the shortest one is padded with spaces until they are of equal length.
This does effectively mean that trailing spaces are useless in regular string comparisons!

This is defined in the comparison operator in the SQL spec:

a) If the length in characters of X is not equal to the length in characters of Y, then the shorter string is effectively replaced, for the purposes of comparison, with a copy of itself that has been extended to the length of the longer string by concatenation on the right of one or more pad characters, where the pad character is chosen based on CS. If CS has the NO PAD attribute, then the pad character is an implementation-dependent character different from any character in the character set of X and Y that collates less than any string under CS. Otherwise, the pad character is a <space>.

The key to successfully exploit the application is resides in this particularity.

The first thing we need to do is create a new user, with the same name as "admin".

We can do this by entering a username which starts with admin, padded with spaces until the column size of 20, and a non-space character.

This will pass the username exists condition, as the given username will not exist.

When inserting the new user, as the column has a 20 character limit, the value will be truncated to 20 characters and therefore be admin, with trailing spaces.
Note that this would not be possible if the column was unique, as we would get a duplicate entry message.

The second step is to login, with the admin username and our created user's password.

As the sql table now has two users, the regular admin and our fake one, the login query will work and select our new user.

Finally, as the application finds the currently logged in user based on its username, it select the first user with the admin username. This means it will select the admin user instead of our own, and we can now steal the admin secret!

Conclusion

Security issues are only bugs in your application.

Like regular bugs, you can limit their quantity by maintaining high quality code.
In this application, marking the username column unique, using the id to identify the current user or performing length validation beforehand would have prevented the issue.

References

Discussion

pic
Editor guide
Collapse
funkybob profile image
Curtis Maloney

I have to ask... which DBMS did you try this with?
Intrigued, I tested your sample 'x' = 'x ' query on Postgres 10.4 and it did not match.

Collapse
antogarand profile image
Antony Garand Author

MySql, MSSql and oracle work from my experience

Collapse
martinmarques profile image
Martín Marqués

I'd be more than surprised if Oracle would not distinguish 'admin' from 'admin ' on insert, but I don't have an Oracle server handy to test. I can assure that this doesn't happen with Postgres (tested on 9.4 - 10)

Very interesting findings. I hope t never bump into issues like that :)

Thread Thread
antogarand profile image
Antony Garand Author

I'm not that familiar with Oracle but based on this sqlFiddle, I can confirm to you that this does work:

sqlfiddle.com/#!4/c0be1c/19813

Thread Thread
scottishross profile image
Ross Henderson

Ran in Oracle SQLDev:

begin
    if 'admin' = 'admin ' then dbms_output.put_line(1);
    else dbms_output.put_line(0);
    end if;
end;

Result: 1

It is correct that Oracle automatically trims leading and trailing spaces.

Edit: I should mention that this is on 18.2

Collapse
polc profile image
Paul Le Corre

Great analysis, thanks for sharing !

Collapse
gwunhar profile image
Mardoch

My only quibble is that this isn't guaranteed to work every time, because you don't have guaranteed row order in an un-ordered SELECT. Otherwise, 👍

Collapse
scottishross profile image
Ross Henderson

Very interesting! Thanks for sharing this.