What is wrong with it?

Ben Nash · October 27, 2013

<?php
session_start();
include("inc/config.php");
if (!isset($_SESSION['user'])) {
header('location:index.php');
}else{

include("inc/user_info.php");

$id = $_GET['id'];
$query = mysql_query("SELECT * FROM users WHERE username = '{$_SESSION['user']}' ");
$d = mysql_fetch_array($query);

echo"
<h3>Send Points</h3>
<form action='send_points.php?id={$_GET['id']}' method='POST' />
Amount <input type='text' name='amount' />
<input type='submit' name='send' value='Send Points' />
</form>
";

if (isset($_POST['send'])) {

if (!isset($_GET['id'])) {
	echo"Invalid User";
}else{

$amount = $_POST['amount'];

 if ($_GET['id'] == $d['id']) {
 	echo"You can't send points to yourself.";
 } 

 elseif (empty($_POST['amount'])) {
 	echo"You must enter how many points you want to send.";
 }

 elseif($_POST['amount'] > $d['cash']) {
	echo"You dont have that much points.";
}else{

   mysql_query("UPDATE users SET points=points+$amount WHERE id = '$id' ");
    mysql_query("UPDATE users SET points=points-$amount WHERE username = '{$_SESSION['user']}' ");
   echo"SENT";

}

}


}

}

?>

Everything works apart from the part where I'm checking if the id is valid. It should say a error message if the id doesn't exist if the user changes the id value in the url like this: send_points.php?id=xxx

ERRORS I'm getting:

( ! ) Notice: Undefined index: id in C:\wamp\www\GameWork\send_points.php on line 10
Call Stack
#	Time	Memory	Function	Location
1	0.0005	254032	{main}( )	..\send_points.php:0

( ! ) Notice: Undefined index: id in C:\wamp\www\GameWork\send_points.php on line 16
Call Stack
#	Time	Memory	Function	Location
1	0.0005	254032	{main}( )	..\send_points.php:0

October 27, 2013

if(isset($_GET['id'))

Ben Nash · October 27, 2013

if(isset($_GET['id'))

<?php
session_start();
include("inc/config.php");
if (!isset($_SESSION['user'])) {
header('location:index.php');
}else{

include("inc/user_info.php");

$id = $_GET['id'];
$query = mysql_query("SELECT * FROM users WHERE username = '{$_SESSION['user']}' ");
$d = mysql_fetch_array($query);

if(isset($_GET['id'])) {
echo"
<h3>Send Points</h3>
<form action='send_points.php?id={$_GET['id']}' method='POST' />
Amount <input type='text' name='amount' />
<input type='submit' name='send' value='Send Points' />
</form>
";
}else{

echo"Invalid User";

}



if (isset($_POST['send'])) {

$amount = $_POST['amount'];

if ($_GET['id'] == $d['id']) {
 	echo"You can't send points to yourself.";
 } 

 elseif (empty($_POST['amount'])) {
 	echo"You must enter how many points you want to send.";
 }

 elseif($_POST['amount'] > $d['cash']) {
	echo"You dont have that much points.";
}else{
mysql_query("UPDATE users SET points=points+$amount WHERE id = '$id' ");
   mysql_query("UPDATE users SET points=points-$amount WHERE username = '{$_SESSION['user']}' ");
   echo"SENT";

}
}

}

?>

New code and it still goes ahead and sends points to users that don't exist

Edited October 27, 2013 by Ben Nash

SRB · October 27, 2013

You're not querying whether the user exists who they wish to send to...

Ben Nash · October 27, 2013

How would I do that Guest?

secret_life · October 27, 2013

Is not an error is a warning he dont bug your code

Another warning : protect your user entry

at least:

$id=abs((int) $_GET['ID']);
$amount=abs((int) $_POST['amount'];

Seker · October 27, 2013

$realuser = mysql_query("SELECT `userid` FROM `users` WHERE `userid` = {$_GET['ID']}", $c);
if (!mysql_num_rows($realuser))
{
    die('Not a real user');
}
else
{
    ....Do Some Stuff....
}

Zettieee · October 27, 2013

This reminds me of grpg...

but here goes:

<?php
session_start();
include("inc/config.php");

if (!isset($_SESSION['user'])) {
	header('location:index.php');
		}else{

include("inc/user_info.php");

$id = abs(intval($_GET['id']));
$query = mysql_query("SELECT * FROM users WHERE username = '{$_SESSION['user']}' ");
$d = mysql_fetch_array($query);

echo"
<h3>Send Points</h3>
<form action='send_points.php?id={$id}' method='POST' />
Amount <input type='text' name='amount' />
<input type='submit' name='send' value='Send Points' />
</form>
";

if (isset($_POST['send'])) {

   if (!isset($_GET['id'])) {
       echo"Invalid User";
   }else{

   $amount = $_POST['amount'];

    if ($_GET['id'] == $d['id']) {
       echo"You can't send points to yourself.";
    } 

    elseif (empty($_POST['amount'])) {
       echo"You must enter how many points you want to send.";
    }

    elseif($_POST['amount'] > $d['cash']) {
       echo"You dont have that much points.";
   }else{

	$check = mysql_query("SELECT * FROM users WHERE id = '".$id."'");
	$run = mysql_num_rows($check);
		if($run == ""){ 	
			echo "Invalid user id.";
			} else {

   mysql_query("UPDATE users SET points=points+$amount WHERE id = '$id' ");
    mysql_query("UPDATE users SET points=points-$amount WHERE username = '{$_SESSION['user']}' ");
   echo"SENT";

   }

}


}

}

?>

Ben Nash · October 28, 2013

<?php
session_start();
include("inc/main.php");
if (!isset($_SESSION['user'])) {
header('location:index.php');
}else{

include("inc/user_info.php");

$id = abs ((int) $_GET['id']);

echo"
<h3>Send Points</h3>
<form action='send_points.php?id={$_GET['id']}' method='POST' />
Amount: <input type='text' name='points' />
<input type='submit' name='send' value='Send Points!' />
</form>
";


if (isset($_POST['send'])) {



$amount = abs ((int) $_POST['points']);

         $query = mysql_query("SELECT * FROM users WHERE username = '{$_SESSION['user']}' ");
         $d = mysql_fetch_array($query);

$exists = mysql_query("SELECT id FROM users WHERE id = '$id' ");
if (!mysql_num_rows($exists)) {
	echo"Invalid User.";
}

elseif (empty($amount)) {
	echo"You need to enter something.";
}

elseif ($amount > $d['points']) {
	echo"Not enough points.";
}

elseif ($id == $d['id']) {
	echo"You can't send points to yourself.";
}else{
	echo"Points Sent!";
	mysql_query("UPDATE users SET points=points+$amount WHERE id = '$id' ");
	mysql_query("UPDATE users SET points=points-$amount WHERE username = '{$_SESSION['user']}' ");
}
   }
}

?>

Working apart from if I change the url to just send_points.php, it stilling saying them errors...

HauntedDawg · October 28, 2013

<?php
session_start();
include("inc/main.php");
if (!isset($_SESSION['user'])) {
header('location:index.php');
}else{

include("inc/user_info.php");

$id = abs ((int) $_GET['id']);

echo"
<h3>Send Points</h3>
<form action='send_points.php?id={$_GET['id']}' method='POST' />
Amount: <input type='text' name='points' />
<input type='submit' name='send' value='Send Points!' />
</form>
";


if (isset($_POST['send'])) {



$amount = abs ((int) $_POST['points']);

         $query = mysql_query("SELECT * FROM users WHERE username = '{$_SESSION['user']}' ");
         $d = mysql_fetch_array($query);

$exists = mysql_query("SELECT id FROM users WHERE id = '$id' ");
if (!mysql_num_rows($exists)) {
	echo"Invalid User.";
}

elseif (empty($amount)) {
	echo"You need to enter something.";
}

elseif ($amount > $d['points']) {
	echo"Not enough points.";
}

elseif ($id == $d['id']) {
	echo"You can't send points to yourself.";
}else{
	echo"Points Sent!";
	mysql_query("UPDATE users SET points=points+$amount WHERE id = '$id' ");
	mysql_query("UPDATE users SET points=points-$amount WHERE username = '{$_SESSION['user']}' ");
}
   }
}

?>

Working apart from if I change the url to just send_points.php, it stilling saying them errors...

If I were in your shoes, id just disable error reporting. Majority of mods copied from here, will report errors.

Since you are not sure how to resolve them, rather just disable the error reporting.

By the way.

$id = abs ((int) $_GET['id']);

===

$id = (array_key_exists('id', $_GET) && ctype_digit($_GET['id'])) ? $_GET['id'] : false;

Razor42 · October 28, 2013

Not to steal the post or anything but HD why would you use that?

Honest question I’m trying to improve my security and wondered what advantages that would give you and when/where it should be used?

Thank you.

October 28, 2013

If I were in your shoes, id just disable error reporting. Majority of mods copied from here, will report errors.
Since you are not sure how to resolve them, rather just disable the error reporting.

By the way.

$id = abs ((int) $_GET['id']);

===

$id = (array_key_exists('id', $_GET) && ctype_digit($_GET['id'])) ? $_GET['id'] : false;

If your using a script where you are disabling the errors, you shouldn't be using that script in the first place :s

Lucifer.iix · November 18, 2013

Notice: Undefined index: id in C:\wamp\www\GameWork\send_points.php on line 10

It means your INDEX in your ARRAY doesn't exists.

So for: MyArray = array('Index1' => 1, 'Index2' => 2); The index 'IdontExisttInThisArray' isn't in your array and there for is complaining.

That is the translation of your ERROR (It's NOT a warning!).

So, that said: If you don't want to show a form when you don't have a id then try branching.

like: If(isset($SomeArray[$SomeIndex])) { echo '<form.....'; } else { throw new HustonWeHaveAProblem("Ohhhh, no !"); }

SRB · November 18, 2013

I was bored, so wrote the code how I feel it should have been wrote to start with.

If anybody uses it, check the mysql calls the correct table/fields in the database and you will need to create the $my variable that contains the players data (Current player you're logged in with)

<?php
session_start();
include("inc/main.php");


// Check user is logged in;
if (!array_key_exists('user', $_SESSION) && is_string($_SESSION['user'])) 
{
   header('location:index.php');
   exit;
}
include("inc/user_info.php");

// Check GET->id is valid (Is set, is string and numeric)
$id = (array_key_exists('id', $_GET) && is_string($_GET['id']) && ctype_digit($_GET['id'])) ? substr($_GET['id'], 0, 12) : FALSE ;
if ($id)
{
   // Check a user exists with the ID;
   $sql = "SELECT `id`,`name` FROM `players` WHERE `id` = '{$id}'";
   $run = mysql_query($sql))
   if (mysql_num_rows($run) == 1)
   {
       // Pull name and id;
       $to = mysql_fetch_assoc($run);


       // Check the data has been posted;
       if ($_SERVER['REQUEST_METHOD'] == "POST")
       {
           // Check amount if above 0;
           $amount = abs ((int) $_POST['points']);
           if ($amount > 0)
           {
               if ($amount > $my['points']) 
               {
                   echo 'You do not have enough points to give that many away.';
               }
               else if ($id == $my['id']) 
               {
                   echo 'How would sending points to yourself benifit you?';
               }
               else
               {
                   mysql_query("UPDATE `users` SET `points` = `points` + $amount WHERE `id` = '$id' ");
                   mysql_query("UPDATE `users` SET `points` = `points` - $amount WHERE `username` = '{$_SESSION['user']}' ");
                   echo 'You sent ' . number_format($amount) . ' points to ' . htmlentities($to['name'], "UTF-8", "ENT_QUOTES") . '!';
               }
           }
       }
       echo '<h3>Send Points</h3>
           Amount: <input type="text" name="points">
           <input type="submit" name="send" value="Send Points!">
       </form>';
   }
   else
   {
       echo 'Invalid user selected.';
   }
}
else
{
   echo 'No valid ID provided.';
}

Lucifer.iix · November 18, 2013

You can also make a procedure or a SQL script that uses transactions. So people can't give point, if they are very fast to two people. After you updated the DB there is always a little bit of time before you update `$my['points']`. And there for give more point than they own. The db value of point will go negative for that user.

Create Procedure `swapPoints` (in fromPK int, in toPK int, in amount int)

-- First check the toPK user exists.... But please don't do this here, but inside the CALLING code, like your test framework or production code.

Start Transaction;

SET @amountAvailable:=0;

-- Create's a LOCK, so `THIS` script will wait until, no-one is writing and than locks the record.

SELECT `amount` INTO @amountAvailable FROM `users` FOR UPDATE

-- We don't want to mutch or negative points. Transfered = [0 -> amountAvailable]

SET @amountTransfered:=Greatest(0,Least(amount, @amountAvailable));

UPDATE `users` SET `points` = `points` - @amountTransfered WHERE `id` = fromPK;

UPDATE `users` SET `points` = `points` + @amountTransfered WHERE `id` = toPK;

Commit Transaction; -- Last thing to do is commit the change...

If you want to make it really secure, for cheating....

Happy Hacking: Roger

Edited November 18, 2013 by Lucifer.iix
I'm also clueless why i did this....

What is wrong with it?

Recommended Posts

Link to comment

Share on other sites

Guest

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Guest

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation