[SOLVED] Msg system

[onthespot]

Hey, I have got the inbox, the send message working. However on the reading messages I am having problems.

 

read_message.php?messageid=23

This is the URL to the page.

 

<?php

include("include/session.php");

$userfinal=$_SESSION['username'];

$user=$userfinal;

$messageid = $_GET['messageid'];

 

$res=mysql_query("SELECT * FROM messages WHERE message_id = '$message_id' AND to_user = '$user'");

 

while($row=mysql_fetch_assoc($res)){

 

$fromuser=$row['from_user'];

$title=$row['message_title'];

$contents=$row['message_contents'];

?>

<table>

<tr><td><?echo "$fromuser\n";?></td></tr>

<tr><td><?echo "$title\n";?></td></tr>

<tr><td><?echo "$contents\n";?></td></tr>

<tr><td><?echo "<br />\n";?></td></tr>

</table>

 

<?

echo '<form name="backfrm" method="post" action="inbox.php">';

echo '<input type="submit" value="Back to Inbox">';

echo '</form>';

?>

 

That is the code for the reading of the messages.

Can anyone see a problem? Thankyou

[ignace]

Can anyone see a problem?

 

Yeah, I see more problems then you are asking for

 

<?echo "$fromuser\n";?>

 

should be:

 

<?="$fromuser\n";?>
//or
<?php echo "$fromuser\n"; ?>

[onthespot]

That didnt work at all, what do you mean you can see more problems?

Any ideas on how this will work?

[ignace]

Do you get any error messages? What is the output you get? Are you sure there are any records at all?

 

what do you mean you can see more problems?

 

try:

 

read_message.php?messageid=1' OR 1=1--

 

Which gives:

 

SELECT * FROM messages WHERE message_id = '1' OR 1=1--' AND to_user = 'someusername'

 

Oh! look at that: Each and every message in the database

[onthespot]

Try this where? I have it working now, the only problem, it isn't pulling the information from the database!

[onthespot]

There are records, my problem is somewhere between my database and the displaying on the fields, its not transferring the data.

[onthespot]

Fixed it, merely change the WHERE and AND around. It worked!

[RestlessThoughts]

I know this topic says solved, but I just had to jump in and mention that, as ignace was trying to point out, there is zero security in this script. :-\

 

Also, in your first post the variables don't match the query statment.

 

Changing this...

$userfinal=$_SESSION['username'];
$user=$userfinal;
$messageid = $_GET['messageid'];

 

to the following would be much safer/better.

 

$user=$_SESSION['username'];
$message_id = trim($_GET['messageid']);
if(!is_numeric($message_id)){
die("Sorry, but that's not a proper message id!");
}

 

This way, your database is better protected and the rest of your code works as written.

[onthespot]

Ok I shal implement this, security is a point I really need to learn more about.

What does trim do? Thankyou for this

[ignace]

$message_id = trim($_GET['messageid']);

 

can also be accomplished by using type conversion:

 

$message_id = (int) $_GET['messageid'];// converts 1' OR 1=1-- to 1

[RestlessThoughts]

Yeah that's true ignace. But I figured using the if statement would make it easier for them to adapt code to other security issues.

 

And trim just makes it so there's no whitespace around the variable, complusive habit of mine to add it in.

 

Good luck with your site onthespot.

[onthespot]

Thanks guys, been a real help there. The whole site coming along nicely now.

[ignace]

And trim just makes it so there's no whitespace around the variable, complusive habit of mine to add it in.

 

It trims more then just whitespace (if the second parameter is left out):

*    " " (ASCII 32 (0x20)), an ordinary space.
* "\t" (ASCII 9 (0x09)), a tab.
* "\n" (ASCII 10 (0x0A)), a new line (line feed).
* "\r" (ASCII 13 (0x0D)), a carriage return.
* "\0" (ASCII 0 (0x00)), the NUL-byte.
* "\x0B" (ASCII 11 (0x0B)), a vertical tab.

 

It's worth noting that you can also add your own characters:

print trim('adbecfa', 'cab'); // dbecf