0

I'm currently creating a ban form that bans users through the MySQLi database, however currently when I try to ban them, the 'active' changes to 0 as opposed to 2 and the banreason does not get updated... Here is the code

$qry = "UPDATE members SET active = '2' AND breason = '".$_POST['reason']."' WHERE login =  '".$_POST['login']."'";
$result = @mysqli_query($GLOBALS["___mysqli_ston"], $qry);
if($result) {
    header("location: banneduser?login=$login&reason=$reason");
    exit();
}else {
    die("Error:".((is_object($GLOBALS["___mysqli_ston"])) ? mysqli_error($GLOBALS["___mysqli_ston"]) : (($___mysqli_res = mysqli_connect_error()) ? $___mysqli_res : false)));
}

I am not receiving any error on the page.

Brad
  • 417
  • 3
  • 18
  • Side question....where are you declaring $login and $reason, they are used as $_POST variables in the statement, and in your header they are just $login and $reason. – Kylie Jun 07 '13 at 22:25
  • Try outputting your query before executing to see if it's correct: `echo $qry;` – showdev Jun 07 '13 at 22:26
  • @KyleK I initially used the variables, and I switched to $_Post wondering if it'd have any effect (I'm a noob when it comes to PHP) – Brad Jun 07 '13 at 22:31
  • @showdev I outputted the query and it displayed this `UPDATE members SET active = '2' AND breason = 'omg' WHERE login = 'Test'` Which is correct – Brad Jun 07 '13 at 22:37
  • 2
    also....just a quick thing to try....take the @ sign off of mysqli_query – Kylie Jun 07 '13 at 22:37
  • There is ah huge security issue with your code, never use $_POST directly in your code like that and learn [How to prevent SQL injection in PHP](http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php). Also, you use the error suppressor operator `@` when calling `mysqli_query()`, not only is this [not recommended](http://stackoverflow.com/questions/136899/suppress-error-with-operator-in-php), it is also detrimental when trying to find where the error is ... Try removing it. – Lepidosteus Jun 07 '13 at 22:40
  • @KyleK Done, but nothing changed. – Brad Jun 07 '13 at 22:40
  • What is the value of `$result` after `mysql_query`? `var_dump($result);` – showdev Jun 07 '13 at 22:41
  • @Lepidosteus Only admins can access this page, I removed it, but no error was echoed; When I do this query, the database updates the user 'Test' (the user im testing it on) with active = '0' and breason is left blank. – Brad Jun 07 '13 at 22:41
  • @showdev "bool(true)" – Brad Jun 07 '13 at 22:44
  • @user2464810: so it's "safely unsafe" ... as long as no other security hole exists in your code. Most exploit work by abusing several exploits one after another. For example, is every single form in your application protected from CSRF ? If not, then a non-admin can abuse your code. Every user input, from an admin or otherwise, should be validated and filtered before use. That's basic security day 1 lesson 1. – Lepidosteus Jun 07 '13 at 22:45
  • sometimes putting single quotes around the field names mysteriously just solves problems for me...... 'active' 'breason'....I know its a silly suggestion, but I would definitely give it a go – Kylie Jun 07 '13 at 22:46
  • @KyleK Simply created a syntax error, thanks for the try though! – Brad Jun 07 '13 at 22:48
  • @user2464810: you completely missed or ignored my point. I'm not sure which one is scarier. – Lepidosteus Jun 07 '13 at 22:50
  • @Lepidosteus There really isn't a security concern if I have a code that does not work, correct? My website is not global, only few know of that (perhaps I should have mentioned this), I only want to set up the basics, if security becomes an issue, I will deal with it in the most appropriate means necessary, however, security is not my main concern because I do not have a code that works correctly. I 100% appreciate your concern for my websites safety, and I'll take note of what you said for **future use**. – Brad Jun 07 '13 at 22:55

1 Answers1

1

Despite the fact that given the comments your code is insecure and you are ok with that, columns to be updated should not be separated by the AND operator in an UPDATE statement.

UPDATE [LOW_PRIORITY] [IGNORE] table_reference
    SET col_name1={expr1|DEFAULT} [, col_name2={expr2|DEFAULT}] ...
    [WHERE where_condition]
    ...

Replace your AND with a comma.

Lepidosteus
  • 11,779
  • 4
  • 39
  • 51