-4

I wrote a code when a user clicks "delete" but it doesn't delete the account, only logs out, I tried searching on internet but nothing was found that helped me. Here's the code:

<?php include('server.php'); 

session_start();

if (isset($_GET['delete'])) {
    $query = "DELETE FROM `users` WHERE `username` = '$username', `password`='$password'";
    mysqli_query($db, $query);
    session_destroy();
    unset($_SESSION['username']);
    unset($_SESSION['password']);
    unset($_SESSION['money']);
    header("location: login.php");
}
?>

Is there any solutions to this code? should I use AND instead of comma, because it didn't work that way, maybe there's an mistake. $query has the same code that was used on phpmyadmin and it was successful there.

Sorry about the server.php Here is the code of it:

Also using md5 for encrypting passwords is not good idea, I probably need to change it.

Here is login.php where the first code came from (the register button is not programmed properly):

<?php include('server.php'); 
 
 session_start();
 
 if (isset($_GET['delete'])) {
  $stmt = $db->prepare('DELETE FROM users WHERE username = ? AND password = ?');
  $stmt->bind_param('ss', $_SESSION['username'], $_SESSION['password']); // 's' specifies the variable type => 'string'
  $stmt->execute();
  session_destroy();
  unset($_SESSION['username']);
  unset($_SESSION['password']);
  unset($_SESSION['money']);
  header("location: login.php");
 }
?>
<!DOCTYPE html>
<html>
<head>
 <title>Login</title>
 <link rel="stylesheet" type="text/css" href="style.css">
 <script>
  function register() {
   header("location: register.php");
  }
 </script>
</head>
<body>
 </br>
 </br>
 </br>
 </br>
 </br>
 </br>
 </br>
 </br>
 
 <form method="post" action="login.php" align="center">

  <?php include('errors.php'); ?>

  <div class="input-group">
   <label>Username</label>
   <input type="text" name="username" >
  </div>
  <div class="input-group">
   <label>Password</label>
   <input type="password" name="password">
  </div>
  <br/>
  <div class="input-group">
   <button type="submit" class="btn" name="login_user">Login</button>
  </div>
  <p></p></br>
  <p>
   <small class="input-group"> Not yet a member? </small> <button type="button" class="btn2" onclick="register()" name="register">Register</button>
  </p>
 </form>

</body>
</html>

This is register.php:

<?php include('server.php') ?>
<!DOCTYPE html>
<html>
<head>
 <title>Registration system PHP and MySQL</title>
 <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
 </br></br></br></br></br></br></br>
 <div class="header">
  <h2>Register</h2>
 </div>
 
 <form method="post" action="register.php">

  <?php include('errors.php'); ?>

  <div class="input-group">
   <label>Username</label>
   <input type="text" name="username" value="<?php echo $username; ?>">
  </div>
  <div class="input-group">
   <label>Email</label>
   <input type="email" name="email" value="<?php echo $email; ?>">
  </div>
  <div class="input-group">
   <label>Password</label>
   <input type="password" name="password_1">
  </div>
  <div class="input-group">
   <label>Confirm password</label>
   <input type="password" name="password_2">
  </div>
  <div class="input-group">
   <button type="submit" class="btn" name="reg_user">Register</button>
  </div>
  <p>
   Already a member? <a href="login.php">Sign in</a>
  </p>
 </form>
</body>
</html>

This is errors.php:

<?php  if (count($errors) > 0) : ?>
 <div class="error">
  <?php foreach ($errors as $error) : ?>
   <p><?php echo $error ?></p>
  <?php endforeach ?>
 </div>
<?php  endif ?>

This is index.php:

<?php 
 session_start(); 

 if (!isset($_SESSION['username'])) {
  $_SESSION['msg'] = "You must log in first";
  header('location: login.php');
 }

 if (isset($_GET['logout'])) {
  session_destroy();
  unset($_SESSION['username']);
  unset($_SESSION['password']);
  unset($_SESSION['money']);
  header("location: login.php");
 }
 
?>
<!DOCTYPE html>
<html>
<head>
 <title>Home</title>
 <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
 <div class="content">

  <!-- notification message -->
  <?php if (isset($_SESSION['success'])) : ?>
   <div class="error success" >
    <h3>
     <?php 
      echo $_SESSION['success']; 
      unset($_SESSION['success']);
     ?>
    </h3>
   </div>
  <?php endif ?>

  <!-- logged in user information -->
  <?php  if (isset($_SESSION['username'])) : ?>
   <p>Welcome <strong><?php echo $_SESSION['username']; ?></strong></p>
   <p> <a href="index.php?logout='1'" style="color: red;">logout</a> </p>
   <p> <a href="login.php?delete='1'" style="color: red;">delete</a> </p>
  <?php endif ?>
 </div>
  
</body>
</html>
vanjmali
  • 1
  • 1
  • 4
    Check for errors after executing sql statements (`mysqli_error`). Also learn about prepared statements to prevent sql injection. **Never** store plain text passwords – Jens Dec 31 '18 at 12:55
  • 4
    *should I use AND instead of comma* YES – Jens Dec 31 '18 at 12:56
  • 1
    Use `AND` when adding multiple conditions, not `, `. Are you sure that these variables used in the query are defined and visible? Also suggests unhashed passwords, which is very bad! Not to mention sql injection vulnerabilities – Qirel Dec 31 '18 at 12:58
  • Also make sure that user making the query have been granted *delete* privilege – J-M.D Dec 31 '18 at 13:02
  • You're not checking for errors from your SQL query. When a query fails, `mysqli_query` will return `false`. Then you use `mysqli_error` to get the error message. There are a variety of reasons why your query could be failing, primarily centered around your SQL injection vulnerability which is not only a security hole but also a *very common* source of bugs. Your query could also be *succeeding* and simply not matching any records. We can't know. You have to debug to find out more. – David Dec 31 '18 at 13:03
  • Change your query as `$query = "DELETE FROM users WHERE username = '$username' AND password='$password'";` – phpfresher Dec 31 '18 at 13:06
  • You don't know what's wrong because you don't check for errors in your code. Never assume the code is always going to work flawlessly. Use [`mysqli_error()`](http://php.net/manual/en/mysqli.error.php) to get a detailed error message from the database. – John Conde Dec 31 '18 at 13:17
  • 1
    **Never store plain text passwords!** Please use **[PHP's built-in functions](//php.net/manual/en/function.password-hash.php)** to handle password security. If you're using a PHP version less than 5.5 you can use the password_hash() **[compatibility pack](https://github.com/ircmaxell/password_compat)**. Make sure you **[don't escape passwords](//stackoverflow.com/q/36628418/1011527)** or use any other cleansing mechanism on them before hashing. Doing so changes the password and causes unnecessary additional coding. – John Conde Dec 31 '18 at 13:17

1 Answers1

0

Without knowing what the content of server.php is, I can see a few error. First, your code is vulnerable for SQL Injections. There are several topics about this on SO, rather read that one here.

Starting with your code - $db,$username, $password are undefined. Guessing from the next lines, it has to be $_SESSION['username'] and $_SESSION['password'] instead.

Also, the SQL doesn't look valid to me, but that's one thing I am not sure about - according to my brain it should be

$stmt = $db->prepare('DELETE FROM users WHERE username = ? AND password = ?');
$stmt->bind_param('ss', $_SESSION['username'], $_SESSION['password']); // 's' specifies the variable type => 'string'
$stmt->execute();

Also I hope you don't store passwords in plaintext.

maio290
  • 6,440
  • 1
  • 21
  • 38
  • I tried a lot of stuff: `$query = "DELETE FROM users WHERE username = '$username'";` `$query = "DELETE FROM users WHERE username = $username";` and also tried with `` in users and username. I also tried your version of it and it wasn't working, I tried `$_SESSION['username']` and that gives me an error. I tried given comments but even info wasn't given. But if I try to use with the name instead of the variable with the name, it worked successfully. Also I updated the question with more codes used. – vanjmali Dec 31 '18 at 16:27