[PHP] Redirecten werkt niet met variabel

[Verwijderd]

Beste devschuur, ik heb een probleem met de code van een login-scherm.
Alles werkt en wilt goed inloggen. Waarna ik de gebruiker wil redirecten naar waar hij vandaan kwam (doormiddel van $_GET['redirect']). Dit werkt niet.

Ik krijg de variabel en kan die ook echo'en (zie lijn 5). Maar als ik hem in de header('location:") wil zetten, dan lijkt de variabel leeg (zie lijn 40).

Hier is de code:

PHP:

<?php
session_start();
$redir = $_GET['redirect'];
//hij print wel gewoon de variable
echo $redir;
    if($_POST) {
        require_once 'misc/php/config.php';
        $username = $_POST['username'];
        $password = $_POST['password'];     
        $conn = mysql_connect($dbhost,$dbuser,$dbpass) or die ('Error connecting to mysql');
        mysql_select_db($dbname);
        $query = sprintf("SELECT COUNT(id) FROM users WHERE UPPER(username) = UPPER('%s') AND password='%s'",
            mysql_real_escape_string($username),
            mysql_real_escape_string(md5($password)));
        $result = mysql_query($query);
        list($count) = mysql_fetch_row($result);
        
        if($count == 1) {
            $_SESSION['authenticated'] = true;
            $_SESSION['username'] = $username;
            $_SESSION['admin'] = false;
            $query = sprintf("UPDATE users SET last_login = NOW() WHERE UPPER(username) = UPPER('%s') AND password = '%s'",
                mysql_real_escape_string($username),
                mysql_real_escape_string(md5($password)));
            mysql_query($query);
            $result = mysql_query($query);
            //registreert het ip-adres
            $last_ip = $_SERVER['REMOTE_ADDR'];
            $query = mysql_query(" UPDATE users SET last_ip = '$last_ip' WHERE username='$username' ") or die(mysql_error()); ;
        
            //checkt of de gebruiker gebanned is
            $result = mysql_query(" SELECT * FROM banned WHERE UPPER(username) = UPPER('$username') ") ;
            $row = mysql_fetch_array( $result );
            if ($row['game_ban'] == '1') { 
                $succes = "Je bent gebanned voor de komende " . $row['banned_for'] . " dagen.</font></b></label>Reden:<br><label><b><font color=white>" . $row['banned_why'] . "</font></b></label><br>";
                $_SESSION['authenticated'] = false;
            }
            else {
            //dit is het stukje code dat niet wil, de variable is hier opeens leeg
            header("location:" . urlencode($redir) . ".php");}
        }
        else { $succes = "Gebruikersnaam of wachtwoord is incorrect.";}
    }
?>

Ik hoop dat jullie zien wat ik fout doe .
Alvast bedankt!

[CyBeR]

euh

PHP:

$redir = $_GET['redirect'];

en dan

PHP:

    if($_POST) {

dat strookt niet met elkaar.

[Room42]

Mwah, een gecombineerde $_POST en $_GET zou nog wel kunnen. De header moet echter "Location: " zijn, dus met een hoofdletter L.

Let wel op dat de redirect die je nu gebruikt potentieel misbruikt kan worden. Je kunt er nu namelijk alles van maken. Gebruik bijvoorbeeld maar eens "http://www.google.com/?random=". Google zal niks met de variabele 'random' doen, dus maakt het ook niet uit dat er .php achter staat. Beter doe je:
a) een check op de waarde van $_GET['redirect']
b) gebruik je een absolute URL

[CyBeR]

Room42 schreef op maandag 29 april 2013 @ 02:42:
Mwah, een gecombineerde $_POST en $_GET zou nog wel kunnen.

Goed, kan inderdaad wel als je POST naar iets.php?met_querystring. Echter zou dan $_GET['blaat'] gewoon netjes vol zitten.

De header moet echter "Location: " zijn, dus met een hoofdletter L.

Headers zijn case insensitive.

[wjzijderveld]

Je kan geen header versturen als er al output is.
Dus zoals je het nu hebt staan, met echo, zal het niet werken. Als je de echo weg haalt, werkt het dan alsnog niet?

[P.O. Box]

lees de definitie van urlencode eens door en je weet wat er mis gaat denk ik

[Verwijderd]

CyBer heeft helemaal gelijk. De variabel verdwijnt natuurlijk. Heel erg bedankt.
Hier ben ik gister veel te laat voor opgebleven.

De nieuwe code:

PHP:

<?php
session_start();
    if($_POST) {
        require_once 'misc/php/config.php';
        $username = $_POST['username'];
        $password = $_POST['password'];     
        $conn = mysql_connect($dbhost,$dbuser,$dbpass) or die ('Error connecting to mysql');
        mysql_select_db($dbname);
        $query = sprintf("SELECT COUNT(id) FROM users WHERE UPPER(username) = UPPER('%s') AND password='%s'",
            mysql_real_escape_string($username),
            mysql_real_escape_string(md5($password)));
        $result = mysql_query($query);
        list($count) = mysql_fetch_row($result);
        
        if($count == 1) {
            $_SESSION['authenticated'] = true;
            $_SESSION['username'] = $username;
            $_SESSION['admin'] = false;
            $query = sprintf("UPDATE users SET last_login = NOW() WHERE UPPER(username) = UPPER('%s') AND password = '%s'",
                mysql_real_escape_string($username),
                mysql_real_escape_string(md5($password)));
            mysql_query($query);
            $result = mysql_query($query);
            //registreert het ip-adres
            $last_ip = $_SERVER['REMOTE_ADDR'];
            $query = mysql_query(" UPDATE users SET last_ip = '$last_ip' WHERE username='$username' ") or die(mysql_error()); ;
        
            //checkt of de gebruiker gebanned is
            $result = mysql_query(" SELECT * FROM banned WHERE UPPER(username) = UPPER('$username') ") ;
            $row = mysql_fetch_array( $result );
            if ($row['game_ban'] == '1') { 
                $succes = "Je bent gebanned voor de komende " . $row['banned_for'] . " dagen.</font></b></label>Reden:<br><label><b><font color=white>" . $row['banned_why'] . "</font></b></label><br>";
                $_SESSION['authenticated'] = false;
            }
            else {header("Location:" . $_SESSION['redir'] . ".php");}
        }
        else { $succes = "Gebruikersnaam of wachtwoord is incorrect.";}}
    else { $_SESSION['redir'] = $_GET['redirect']; }
?>

Room42 schreef op maandag 29 april 2013 @ 02:42:
Mwah, een gecombineerde $_POST en $_GET zou nog wel kunnen. De header moet echter "Location: " zijn, dus met een hoofdletter L.

Let wel op dat de redirect die je nu gebruikt potentieel misbruikt kan worden. Je kunt er nu namelijk alles van maken. Gebruik bijvoorbeeld maar eens "http://www.google.com/?random=". Google zal niks met de variabele 'random' doen, dus maakt het ook niet uit dat er .php achter staat. Beter doe je:
a) een check op de waarde van $_GET['redirect']
b) gebruik je een absolute URL

Op welke manier zou er misbruik van gemaakt kunnen worden? Ze kunnen toch alleen naar de pagina's die ze normaal ook al kunnen bezoeken?

Voor de rest zijn er meer flaws in deze code hoor. Zoals het niet gebruiken van een salt. Maar dit komt nog .

[ Voor 91% gewijzigd door Verwijderd op 29-04-2013 11:48 ]

[DanielG]

Verwijderd schreef op maandag 29 april 2013 @ 11:39:
CyBer heeft helemaal gelijk. De variabel verdwijnt natuurlijk. Heel erg bedankt.
Hier ben ik gister veel te laat voor opgebleven.

De nieuwe code:

PHP:

$query = mysql_query(" UPDATE users SET last_ip = '$last_ip' WHERE username='$username' ") or die(mysql_error()); ;
        
$result = mysql_query(" SELECT * FROM banned WHERE UPPER(username) = UPPER('$username') ") ;

Voor de rest zijn er meer flaws in deze code hoor. Zoals het niet gebruiken van een salt. Maar dit komt nog .

De SQL injections zijn wellicht belangrijker dan geen salt gebruiken

[4Real]

mysql_real_escape_string wordt trouwens uitgefaseerd. Vanaf PHP 5.5 zal deze er niet meer inzitten. Aangeraden wordt om naar PDO of MySQLi te gaan kijken.

[Verwijderd]

DanielG schreef op maandag 29 april 2013 @ 11:54:
[...]


De SQL injections zijn wellicht belangrijker dan geen salt gebruiken

Ik snap niet waarom het nodig is om bij die code af te gaan vangen. De gebruiker komt daar namelijk niet bij.

4Real schreef op maandag 29 april 2013 @ 12:44:
mysql_real_escape_string wordt trouwens uitgefaseerd. Vanaf PHP 5.5 zal deze er niet meer inzitten. Aangeraden wordt om naar PDO of MySQLi te gaan kijken.

Bedankt voor de tip!

[Cartman!]

Verwijderd schreef op maandag 29 april 2013 @ 19:44:
Ik snap niet waarom het nodig is om bij die code af te gaan vangen. De gebruiker komt daar namelijk niet bij.

Los van of een gebruiker er bij kan moet je gewoon altijd uitgaan van de beste veiligheid. De $_SERVER superglobal wordt grotendeels gevuld door de client, daar kun je ook niet vanuit gaan. Ook zijn een paar queries wat gek... je gebruikt meerdere updates om het record weer te updaten terwijl dat makkelijk in 1 kan. Ook gebruik je de username om te updaten (opzich niet verkeerd) maar ik zou je aanraden om lekker het ID te pakken omdat het minder foutgevoelig is en vaak sneller is omdat het de primary key is.