Condition not working as expected

Question

I have been writing a script for my online game server, to fetch the user name from the db and check for its level.

The problem is that the code doesn't check for the level so any one at any level can vote and abuse my voting system.

NOTE: That voting system is based on time/date so you can only vote once every 12 hrs.

Form code:

<html>
<body>
<center>
Please Enter Your Character Name Below, <br /><br />
After You Vote Please Relogin And Your Cps Will be Added<br /><br />
<FORM action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
  Character Name: <br /><br /> <input type="text" name='CharName'><br>
<br />
  <input type="submit" name="button" value="Vote">
</form>
</center>
</body>
</html>

Vote code:

<html><center>
<?php
$user = 'test'; //dbuser
$pass = 'test'; //dbpass
$host = 'localhost';    //dbhost
$name = 'zf'; //dbname

$con = mysql_connect($host, $user, $pass);
mysql_select_db($name, $con);

$datetime = date('Y-m-d');
$ip = $_SERVER['REMOTE_ADDR'];

if (isset($_POST['button']))
{
    $result1 = mysql_query("SELECT `level` FROM `cq_user` WHERE `name` = '$char_name'") or die(mysql_error());
    while($row = mysql_fetch_array($result1))
    {
    }

    error_reporting(E_ALL);
    ini_set('display_errors', '1');

    $char_name = $_POST['CharName'];

    $result = mysql_query("SELECT name FROM cq_user WHERE name = '" . $char_name . "' AND UNIX_TIMESTAMP(lastvoted) <= UNIX_TIMESTAMP('" . date('Y-m-d H:i:s', strtotime('-12 Hours')) . "')") or die(mysql_error());
    $result1 = mysql_query("SELECT `level` FROM `cq_user` WHERE `name` = '" . $char_name. "'") or die(mysql_error());
    while($row = mysql_fetch_array($result1))
    {
    }

    if (mysql_num_rows($result) == 0 && $row <= 119)
        echo "This character does not exist, or you have entered the wrong name. Or you could be trying to cheat and have already voted. Or you are not level 120+.";
    else
    {
        mysql_query("UPDATE `cq_user` SET `emoney` = `emoney` + 100000, `lastvoted`='" . date('Y-m-d H:i:s') . "' WHERE `name` = '" . $char_name . "'") or die(mysql_error());
        mysql_query("UPDATE `cq_user` SET `ip` = '$ip' WHERE `name` = '$char_name'");
?>
        <meta http-equiv="REFRESH" content="0;url=http://www.xtremetop100.com/in.php?site=1132303596"></HEAD>
<?php
    }
}
?>
</html></center>

Thats my checker

if (mysql_num_rows($result) == 0 && $row <= 119)

That's the part where my Level checker should be working the <= 119!

Isn't that supposed to be `if (mysql_num_rows($result1) == 0 && $row <= 119)`? — Funk Forty Niner, Jul 21 '13 at 03:22
`I cant add the code here , The code posting system is kinda annoying , Confusing , so i posted it on pastebin , hope that this isnot illegal !` not posting the code here reduces drastically your chance of getting good answers as not everyone cares to open secondary pages or simple don't like following secondary pages for security reasons. — Prix, Jul 21 '13 at 03:24
You have a mix of `$result` and `$result1` queries, or is that intentional? — Funk Forty Niner, Jul 21 '13 at 03:25
Well mysql_num_rows($result) == 0 checks if the user already exits or no so if the mysql_num_rows returns 0 then it will fetch the error .. that the user doesnt exist — Ahmed Magdy, Jul 21 '13 at 03:25
No i didnt , And i have updated my question with a view of the code instead of pastebin , im totaly confused of what to do with the script — Ahmed Magdy, Jul 21 '13 at 03:30
And have you tried `if (mysql_num_rows($result1) == 0 && $row <= 119)`? Using `$result1` instead of `$result`? — Funk Forty Niner, Jul 21 '13 at 03:32
@AhmedMagdy [**you're also vulnerable to XSS attacks**](http://stackoverflow.com/questions/6080022/php-self-and-xss) — Prix, Jul 21 '13 at 03:41
I think that i have fixed the vulnerability already , thanks for telling me — Ahmed Magdy, Jul 21 '13 at 04:18

srakl · Accepted Answer · 2013-07-21T06:15:15.217

1

change it to this, it's an associate array.

  if (mysql_num_rows($result) == 0 || $row['level'] <= 119)

also if you $result1 query is returning 1 row, you don't need a while loop here.

while($row = mysql_fetch_array($result1))
    {
    }

change it this

list($row) = mysql_fetch_array($result1);

EDITED

<?php
$user = 'test'; //dbuser
$pass = 'test'; //dbpass
$host = 'localhost';    //dbhost
$name = 'zf'; //dbname

$con = mysql_connect($host, $user, $pass);
mysql_select_db($name, $con);

$datetime = date('Y-m-d');
$ip = $_SERVER['REMOTE_ADDR'];

if (isset($_POST['button']))
{
    $char_name = $_POST['CharName'];
    $result = mysql_query("SELECT `name`, `level` FROM `cq_user` WHERE `name` = '".$char_name."' AND UNIX_TIMESTAMP(lastvoted) <= UNIX_TIMESTAMP('" . date('Y-m-d H:i:s', strtotime('-12 Hours')) . "')") or die(mysql_error());

    list($name, $level) = mysql_fetch_array($result1);

    error_reporting(E_ALL);
    ini_set('display_errors', '1');

if (mysql_num_rows($result) == 0 || $level <= 119)
        echo "This character does not exist, or you have entered the wrong name. Or you could be trying to cheat and have already voted. Or you are not level 120+.";
    else
    {
        mysql_query("UPDATE `cq_user` SET `emoney` = `emoney` + 100000, `lastvoted`='" . date('Y-m-d H:i:s') . "', `ip` = '".$ip."' WHERE `name` = '" . $char_name . "'") or die(mysql_error());
?>
        <meta http-equiv="REFRESH" content="0;url=http://www.xtremetop100.com/in.php?site=1132303596"></HEAD>
<?php
    }
}
?>

your html

<form name="FORMNAME" action="submit.php" method="post">
    <input type="text" name="CharName"  />
    <input type="submit" name="button" value="Submit" />
</form>

edited Jul 21 '13 at 06:15

answered Jul 21 '13 at 03:57

srakl

2,565
2
21
32

When i added the list($row) = mysql_fetch_array($result1); i found 3 HTML forms wrote instead of one , And when i only added the if (mysql_num_rows($result) == 0 && $row['level'] <= 119) it didnt work still – Ahmed Magdy Jul 21 '13 at 04:02
if you `echo $result.' '$row['level'];` what do you get? – srakl Jul 21 '13 at 04:23
I think it works , but idk why the checker isnot working .. Is there a way we can move this conversation to private such as msn , skype etc . – Ahmed Magdy Jul 21 '13 at 04:30
before your `if (isset($_POST['button']))` put this `print_r($_POST);` is "button" there? – srakl Jul 21 '13 at 04:32
Array ( ) Thats what the page printed out – Ahmed Magdy Jul 21 '13 at 04:38
How can i fix that o.O :S ? – Ahmed Magdy Jul 21 '13 at 04:40
also `$result1 = mysql_query("SELECT `level` FROM `cq_user` WHERE `name` = '$char_name'") or die(mysql_error());` i don't see $char_name anywhere before that line. – srakl Jul 21 '13 at 04:40
your form that posts to this page has to have an `` only then will everything in your if statement work – srakl Jul 21 '13 at 04:43
$char_name = $_POST['CharName']; Its under that o.o – Ahmed Magdy Jul 21 '13 at 04:43
Putted it above it , It still doesnt work , Do i have to just copy/paste that for the $_POST to work ? – Ahmed Magdy Jul 21 '13 at 04:46
the input tag goes in your html form – srakl Jul 21 '13 at 05:01
After adding the tag to my HYML form the script should work ? – Ahmed Magdy Jul 21 '13 at 05:42
it should, by the looks of things, you are not posting "button" to you php code, for everything in you if statement to work. – srakl Jul 21 '13 at 06:01
Is that module in your reply is the working version of my script ? – Ahmed Magdy Jul 21 '13 at 06:12

Kyle · Answer 2 · 2013-07-22T19:10:34.627

0

There are a few things to check out here. I'm assuming character names are unique. If so, I'm guessing you're expecting mysql_fetch_array() to return one result for

mysql_query("SELECT `level` FROM `cq_user` WHERE `name` = '" . $char_name. "'")

You have $row = mysql_fetch_array($result1), and as the function name implies, mysql_fetch_array() returns an array, so $row is an array of the column values requested. Each time it's called, it iterates further over the results. If you're just expecting one row to be returned, you can call it just once (no need for the while loop). Since you're just selecting one column (level), the level should be $row[0].

Also, the conditions in your if statement are mutually exclusive:

if (mysql_num_rows($result) == 0 && $row <= 119)

I'm guessing you meant to use or (||) here, because you want to check if either there are 0 results or the level is less than 119.

Therefore, it should likely be:

if (mysql_num_rows($result) == 0 || $row[0] <= 119)

Also, the mysql_ functions are now deprecated. It is suggested you use mysqli_ function or the PDO_MySQL extension. Your code might also be vulnerable to SQL injections as you're not escaping user input before concatenating it with your query string. Consider using prepared/parameterized queries.

edited Jul 22 '13 at 19:10

answered Jul 21 '13 at 03:32

Kyle

2,822
2
19
24

Not to mention that he have 3 queries that could have been resumed into a single query. – Prix Jul 21 '13 at 03:34
So i should edit my if (mysql_num_rows($result) == 0 && $row <= 119) to be if (mysql_num_rows($result) == 0 || $row(0) <= 119) Is that correct? – Ahmed Magdy Jul 21 '13 at 03:35
@Prix: Okay, so there are *at least* a few things to check out. – Kyle Jul 21 '13 at 03:35
Yes, but `$row[0]`, not `$row(0)`. `$row` is an array, so you'll want to use brackets to access an array index, not parentheses. – Kyle Jul 21 '13 at 03:38
When its || IT doesnt let any one vote , even if his level passes 119 , When i set it back to && and row[0] the checker doesnt work so any one at any level can still vote :S im confused – Ahmed Magdy Jul 21 '13 at 03:43
Does the character you're testing for exist, and if so, is its level greater than 119? – Kyle Jul 21 '13 at 03:51
Am i still getting any help here ? – Ahmed Magdy Jul 21 '13 at 04:12
Is anything actually being returned for the query associated with `$result`? – Kyle Jul 21 '13 at 04:16

Condition not working as expected

2 Answers2