-2

I'm trying to do a change user info part on my website, everything works except the change username part.

In this part I have to check if the newUsername is not taken and then change it (if its not already taken).

When I launch my code it doesn't enter the (else if) statement after the (echo "username deja utiliser").

//change.php

$newUsername = $_POST['changeUsername'];
$newNom = $_POST['changeNom'];

$currentId = $_SESSION['id'];
$currentName = $_SESSION['name'];

//fonction that doesn't work
if(empty($newUsername)){
    echo "username is empty";   
    }else if ($stmt = $con->prepare('SELECT username from users where USERNAME = ? ')) {
    $stmt->bind_param('s', $newUsername);
    $stmt->execute();
    $stmt->store_result();
    if ($stmt->num_rows > 0) {
        $stmt->bind_result($checkUsername);
        $stmt->fetch();
        if ($checkUsername === $newUsername) {
           echo "username deja utiliser";
        }else if($stmt = $con->prepare("UPDATE users SET USERNAME = '$newUsername' WHERE ID = ? ")){
            $stmt->bind_param('s', $currentId);
            $stmt->execute();
            echo "les valeur on été modifier";
            session_regenerate_id();
            $_SESSION['name'] = $newUsername;
        }else{
            echo "les valeurs n'ont pas été modifier";
        }
    }
}

//function that works
if(empty($newNom)){
    echo "nom is empty";
}else if($stmt = $con->prepare("UPDATE users SET NOM = '$newNom' WHERE ID = ? ")){
    $stmt->bind_param('s', $currentId);
    $stmt->execute();
    echo "les valeur on été modifier";
}else{
    echo "les valeurs n'ont pas été modifier";
}
    //editProfile.php

    <form action="change.php" method="POST">
     
      <table>
        <tr>
          <td>Username:</td>
          <td>
            <label for="username"></label>
            <input type="text" name="changeUsername" placeholder="<?php echo $_SESSION['name'] ?>" id="changeUsername">
          </td>
        </tr>
        <tr>
          <td>Nom:</td>
          <td>
            <label for="nom"></label>
            <input type="text" name="changeNom" placeholder="<?php echo $nom ?>" id="changeNom">
          </td>
        </tr>
      </table>
    <input type="submit" value="modfier">
    </form>
Community
  • 1
  • 1
Jermey
  • 12
  • 7
  • **Warning!** You're wide open to SQL injection attacks since you're not using prepared statements properly. Yous shouldn't inject _any_ variables directly into your queries. You're, for some reason, mixing injecting variables and using placeholders (the `?`). You should use placeholders for _all_ parameters in _all_ queries. You should also use the correct types. I'm assuming that `$currentId` is an integer, so `$stmt->bind_param('s', $currentId);` should be `$stmt->bind_param('i', $currentId);` – M. Eriksson Oct 26 '21 at 09:39
  • Thanks Magnus, you're right for the ($currentId), I changed it. But could you please explain me how I could correct my prepared statements and also how I could correct my error ? – Jermey Oct 26 '21 at 09:45
  • Ah, and you started so well with your first prepared statement; only to make the second and third prepared statements insecure. – mickmackusa Oct 26 '21 at 09:45
  • You already know how to parameterized the values. Just make sure you do that for all variables you use in the queries: `USERNAME = ? WHERE ID = ?` and then add more `->bind_param(...)` in the correct order. – M. Eriksson Oct 26 '21 at 09:48
  • What are the input values when your script fails to work as desired? – mickmackusa Oct 26 '21 at 09:50
  • @MagnusEriksson Thanks a lot, I just saw my error... But do you know why the first function doesn't works ? – Jermey Oct 26 '21 at 09:51
  • @mickmackusa so the script fails when every statement is "true". It fails when it should change the value of the username and all the other params are OK. – Jermey Oct 26 '21 at 09:53
  • 1
    I assume that your first query should just be checking for a `COUNT(1)` value of one instead of bothering to bind a name value for comparison. https://stackoverflow.com/a/9406106/2943403 Regarding `if ($checkUsername === $newUsername) {`, my French is a little rusty -- should this be checking if the `$newUsername` is the same as the name in `$_SESSION['name']`? ...shouldn't that query-less check come before the first query? – mickmackusa Oct 26 '21 at 09:58
  • @mickmackusa yes basically it should do this. It would be a lot easier your right. Did not think about that thanks :) – Jermey Oct 26 '21 at 10:02

1 Answers1

0

Finally I found something that works. Is this code secure for SQL injection ?

//check if the username is taken 
if(empty($newUsername)){
    echo "username is empty";
}else if ($stmt = $con->prepare('SELECT username from users where USERNAME = ? ')) {
    $stmt->bind_param('s', $newUsername);
    $stmt->execute();
    $stmt->store_result();
    if ($stmt->num_rows > 0) {
        $stmt->bind_result($username);
        $stmt->fetch();
        if ($username === $newUsername) {
            $checkUsername = true;
        }
    }
}

//change the username
if (empty($newUsername)) {
    echo "username is empty";
} else if ($checkUsername != true) {
    if ($stmt = $con->prepare("UPDATE users SET USERNAME = ? WHERE ID = ? ")) {
        $stmt->bind_param('si', $newUsername, $currentId);
        $stmt->execute();
        echo "les valeur on été modifier";
        session_regenerate_id();
        $_SESSION['name'] = $newUsername;
    } else {
        echo "les valeurs n'ont pas été modifier";
    }
} else {
    echo "username deja utiliser ";
}
Jermey
  • 12
  • 7
  • It looks like you are asking a new question about SQL injection; if so, please ask about if your code is safe from SQL injections as a separate question. – Shane Bishop Oct 27 '21 at 15:10