0

I've tried to be as specific as possible, but I'm sorry that the subject of my question may be broad.

I got used to a habit of sending variables using the $_GET['variable'], for instance, let's say I'm using ajax to get some information from a database, I would probably do something like this:

xmlhttp.open("GET","read.php?option=someoption",true);

And then I would set the PHP page in a way that behave differently according to the $_GET['option'] it would receive. I then realised that any logged-in user could type the URL and directly modify the database, so I've set some additional $_SESSION['redirect'] variables before each redirection to help prevent access to php pages from URL. By doing a quick ajax call to a "prevent.php" page that would do something like so:

$_SESSION['redirect'] = "true";
header("Location: page.php");

And then having it set this way in the page.php for instance:

if ($_SESSION['redirect']==true) {
    // access the database
}
else {
   // deny access
} 

Is this a reliable way of doing things, or is there a more professional way to sort it out?

James
  • 4,644
  • 5
  • 37
  • 48
  • 1
    As a general rule, `GET` is used to retrieve information, `POST` is used to modify it. – BenM Aug 26 '17 at 16:15
  • 1
    No, thats not a secure way of doing it. – Marco Aug 26 '17 at 16:18
  • 1
    You can do two things: 1.) Save the permissions in the session when the user logs in or 2.) Get the permissions when the user wants to execute an action from a database or file. Of course for the second method you will have to save the user id / username in the session beforehand – Marco Aug 26 '17 at 16:19
  • You could do it like If the user is logged in && has admin rank, which should be set from the database. Then blabla..! – Mr Pro Pop Aug 26 '17 at 16:23
  • Use RBAC of sorts, `http://phprbac.net/` so easy to add :s – Lawrence Cherone Aug 26 '17 at 16:26
  • Post, get, session and cookie can all be created or edited by the user so these should not be anything to rely on when your talking about security, always clean those values before using them in your script or database, if you want to use it for security then store multiple values wich you will compare to database data to verify any security issue. – K. Tromp Aug 26 '17 at 21:17

1 Answers1

0

No it's not a secure way of doing it.

Here's an example of how you could achieve a secure user system in the simplest of forms:

login.php

<?php
session_start();

$user = isset($_POST['username']) ? $_POST['username'] : false;
$pass = isset($_POST['password']) ? $_POST['password'] : false; 

# Check credentials
if (validCredentials($user, $pass)) {
    # Credentials are valid

    # Set isAdmin session for the user 'admin'
    # This is hardcoded for simplicity but 
    # you could read a value from a database
    # and set this session dynamically.
    if ($user === 'admin') {
        $_SESSION['isAdmin'] = true;
    }

    # Generate CSRF token (see appendix)
    # heads up: random_bytes() is a PHP7 function.
    $_SESSION['token'] = bin2hex(random_bytes(32));

    # Set logged in session for every user
    $_SESSION['user'] = $user;

    echo 'Successfully logged in!<br />';
    echo '<a href="user-page.php">Go to the user page.</a>';
}

admin-page.php:

<?php
session_start();

if (isset($_SESSION['isAdmin']) && $_SESSION['isAdmin'] === true) {
    echo 'Only the admin can see this.';
} else {
    echo 'You are either not logged in or you don\'t have the permission to view this page.';
}

user-page.php:

<?php
session_start();

if (isset($_SESSION['user'])) {
    $token = $_SESSION['token'];

    echo 'Only logged in users can see this. <br />';
    echo '<a href="safe-logout.php?token='.$token.'">Log me out</a>.';
} else {
    echo 'You are not logged in.';
}

Appendix:

Make sure that you protect yourself against CSRF attacks:

For example an insecure way of logging an user out would be:

logout.php:

<?php
session_start();

if (isset($_SESSION['user'])) {
    session_destroy();
}

Maybe you ask yourself why this is not secure.

The reason is because for every user the logout link is the same (example.com/logout.php).

So it's not hard at all to guess the logout link (well, we don't even have to guess, we already know for sure).

An attacker could disguise the logout link and as soon as you click on it you would be logged out.

It's very important to understand that the logout is just one example.

Think of a more severe action like deleting a user etc.

So this concept applies to every action an authenticated user can do.

To be safe, you can generate a token as soon as the user has logged in.

For every action taken you then check if the token in the request matches the one you generated.

This way the logout link is unique for every user (example.com/loogut.php?token=random_token_generated_at_login) and is only hardly guessable by an attacker.

safe-logout.php:

<?php
session_start();

if (isset($_SESSION['user'])) {
    # Check if the user specified token matches ours
    $token = isset($_GET['token']) ? $_GET['token'] : false;

    if ($_SESSION['token'] === $token) {
        session_destroy();

        echo 'Successfully logged out!';
    } else {
        # We dont logout because the token was not valid
    }
}

And NO: a POST request is just as susceptible as a GET request.

So make sure you check the token for every action, regardless of the HTTP method used.

Marco
  • 7,007
  • 2
  • 19
  • 49
  • No... Why do you use unique for generating random session tokens, and why do you hash it, especially with md5. Just use random_bytes() It generates secure cryptographical bytes, doesn't even need to be hashed and it is hard to be predicted. Let's do some math, a 32-byte token can represent 1E77 combinations. If we assume that the hash can be calculated with 100Giga hashes per second, we can still expect 1.8E58 years to find the hash, a number with 59 digits! Of course, this only applies to truly random tokens. – Mr Pro Pop Aug 26 '17 at 16:48
  • hah, I knew I would get this sort of comment. Feel free to improve my answer. – Marco Aug 26 '17 at 16:49
  • Added a note that `random_bytes` is a PHP7 function — I didn't even knew it existed until now :) Thanks! – Marco Aug 26 '17 at 16:54
  • Ahh, that's good, and openssl_random_pseudo_bytes is for PHP version above 5.3.0 – Mr Pro Pop Aug 26 '17 at 16:58
  • Anyways, if the token doesn't match, we don't log the user out, so what do we do? I am really curious! – Mr Pro Pop Aug 26 '17 at 17:05
  • As a note: this is just a **minimal** example. If you wanted it really secure you'd want to regenerate the session id after the login (`session_regenerate_id()`) as well. @MrProPop Most sites display a `"An error occurred please try again."` or something like that. I intentionally left it out for simplicity :D – Marco Aug 26 '17 at 17:07
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/152907/discussion-between-mr-pro-pop-and-d3l). – Mr Pro Pop Aug 26 '17 at 17:11
  • I'm very grateful to you sir for all this precisous knowledge, i've also been using the md5 encryption method for my passwords in the database, but never heard of random_bytes before, awesome. This is great i'm very excited to implemant all this in my website, thanks a lot ! – Physics Aug 26 '17 at 17:13
  • 2
    Your welcome, hope you have your website secure now, and anyway, for passwords, you should be using password_hash, for random_tokens use random_bytes(), for other things like a persistent_login cookie or password reset link where you pass the token client side, you need this random_bytes to be hashed, for example, with hash('sha256'...) to verify it, in case of user editing. Good luck! – Mr Pro Pop Aug 26 '17 at 17:22
  • Putting tokens in header is not secure – Mr Pro Pop Aug 28 '17 at 21:21
  • @MrProPop What do you mean? – Marco Aug 28 '17 at 21:34
  • 1
    One of the comments says, "I think one of the main disadvantages of using CSRF-token in GET requests is a possibility of an incompetent user to easily disclose his token by copying a link with the token and paste it in some public content like a comment/post/etc... Also GET query parameters including CSRF-tokens usually logged by HTTP servers/proxies and it introduces another risk." Link can be found here https://stackoverflow.com/questions/23966927/is-putting-token-in-url-secure-to-prevent-csrf-attacks-in-php-applications – Mr Pro Pop Aug 28 '17 at 21:36
  • @d3L Nevermind, just forget you are still safe and secure. Found a good method to secure it even if it is passed through GET Requests. – Mr Pro Pop Aug 29 '17 at 00:46