views:

135

answers:

7

This is probably easy for you guys, but I can't understand it. I want to save the filename of an image to it's own row in the SQL base.

Basically, I log on to the site where I have my own userID. And each user has its own column for background images. And the user can choose his own image if he wants to. So basically, when the user clicks on the image he wants, a jquery click event occurs and an ajax call is made to a php file which is supposed to take care of the actual update. The row for each user always exist so there's only an update of the data that's necessary.

First, I collect the filename of the css property 'background-image' and split it so I get only the filename. I then store that filename in a variable I call 'filename' which is then passed on to this jQuery snippet:

    $.ajax({
        url: 'save_to_db.php',
        data: filename,
        dataType:'Text',
        type: 'POST',
        success: function(data) {
            // Just for testing purposes.
            alert('Background changed to: ' + data);
   }   

  });

And this is the php that saves the data:

<?php 
require("dbconnect.php");

$uploadstring = $_POST['filename'];

mysql_query("UPDATE brukere SET brukerBakgrunn = '$uploadstring' WHERE brukerID=" .$_SESSION['id']);
mysql_close();  
?>

Basically, each user has their own ID and this is called 'brukerID' The table everything is in is called 'brukere' and the column I'm supposed to update is the one called 'brukerBakgrunn'

When I just run the javascript snippet, I get this message box in return where it says:

Background changed to:
Warning: session_start() [function.session-start]: Cannot send session cache limiter - headers already sent (output started at /var/www/clients/client2/web8/web/save_to_db.php:1) in /var/www/clients/client2/web8/web/access.php on line 3

This is dbconnect.php

<?php
$con = mysql_connect("*****","******","******");
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }  

mysql_select_db("****", $con);
require("access.php");
?>

And this is access.php:

<?php
// Don't mess with ;)
session_start();

if($_REQUEST['inside']) session_destroy();

session_register("inside");
session_register("navn");
if($_SESSION['inside'] == ""){
    if($_POST['brukernavn'] and $_POST['passord']){
    $query = "select * from brukere where brukerNavn='" . $_POST['brukernavn'] . "' and brukerPassord = md5('" . $_POST['passord'] ."')";
    $result = mysql_query($query);      
    if(!$result) mysql_error();
    $rows = @mysql_num_rows($result);
        if($rows > 0){
    $_SESSION['inside'] = 1;
    $_SESSION['navn'] = mysql_result($result,"navn");
    $_SESSION['id'] = mysql_result($result,"id");
    Header("Location: /");
    } else {
    $_SESSION['inside'] = 0;
    $denycontent = 1;
    }
    } else {
    $denycontent = 1;
    }
}

if($denycontent == 1){
include ("head.php");
print('   
<body class="bodylogin">
   content content content      
</body>
');
include ("foot.php");
exit;
}
?>
+2  A: 
mysql_query("UPDATE brukere SET brukerBakgrunn = $uploadstring WHERE brukerID=" .$_SESSION['id'] ."";

should be

mysql_query("UPDATE brukere SET brukerBakgrunn = $uploadstring WHERE brukerID=" .$_SESSION['id']);

closing parenthesis is missing and the quotes ("") are useless.

Read about SQL injection in order to make your application safe.

EDIT:

<?php
 require("dbconnect.php")
?>

<?php

This code sends (the part between ?> and <?php) a newline to the output (it's the same as echo "\n") which is not allowed if you want to write to a session variable consequently.

MartyIX
+1, I edited to fix your typo and wanted to comment about injection. But you already put the injection thingie in and accidentally reverted my correction ;-)
Michael Krelin - hacker
Too fast collaboration :-)
MartyIX
Ok, I updated the first post. I get a different message from the ajax method. It's about hos it cannot send session cache limiter, header already sent. And the data isn't updated either. And if I remove the 'require("dbconnect.php") from the php file, all I get in return from ajax is nothing. So it looks like it's needed.
Kenny Bones
Remove the extra ?> <?php part from your code as I wrote in my editted answer. I hope it will help.
MartyIX
Ah! Ok, so this removed that error message and gave me another :pCheck my updated first post
Kenny Bones
$uploadstring = ($_POST['filename']); - I'm not sure for what reason you did wrap the variable with parenthesis. $uploadstring = $_POST['filename']; should be ok.
MartyIX
Man, I'm so confused with the syntax.. Thanx, I'll try it! :)Edit: Didn't change anything actually. I still get the exact same error message. At the same line.
Kenny Bones
require("dbconnect.php") <- you miss the semicolon after the command.
MartyIX
UPDATE brukere SET brukerBakgrunn = $uploadstring should be UPDATE brukere SET brukerBakgrunn = '$uploadstring'. Without apostrophes it is not a valid string for mysql (how would it know what is string and what is part of sql syntax?)
MartyIX
Ok, now I added both the ';' after require dbconnect.php AND added the single quotes around the variable. And now that 'cannot send session cache limiter' appear again. This is very confusing.
Kenny Bones
Show us your dbconnect.php script (without credentials) and maybe we will be able to help you.
MartyIX
Just updated :)
Kenny Bones
?> <-- don't use these marks at the end of files (remove them from dbconnect.php and access.php in your case) - you may have some accidental output after these marks.
MartyIX
if($_REQUEST['inside']) session_destroy(); -- the warning message mentioned this line so try to leave it out and see what happens.
MartyIX
Tried removing the session destroy line and that didn't change anything. I'm gonna try removing the ?> from the files that are included by code. Edit: Line 3 in access.php is really just the "session_start();"
Kenny Bones
Try to move the session_start(); before require("dbconnect.php");
MartyIX
Do you mean to move session_start(); or add another in save_to_db.php?I can't move session_start() from access.php can I? Cause I need that to access index.php in the first place. Or do I misunderstand?
Kenny Bones
A: 

you forgot the closing ')' in your mysql_query line !

mysql_query("UPDATE brukere SET brukerBakgrunn = $uploadstring WHERE brukerID=" .$_SESSION['id'] );

You don't need the ."" at the end of your query too.

Rodolphe
+2  A: 

Big security issue!

You didn't quote and escape the input to the MySQL query. I could easily hack the end, stack another query, and delete your entire database!

Also, you're missing the ending parenthesis at the end of mysql_query().

Delan Azabani
And how will you put your query in his Session variable ID? That might be checked and untainted.
Konerak
Why session? uploadstring comes from post data.
Michael Krelin - hacker
So? Even the `$uploadString` isn't quoted + escaped; I could easily hack that one.
Delan Azabani
Never heard of quote and escape the input. Can it easily be fixed?
Kenny Bones
You need to put quotes around the input (to allow spaces) then backslash escape any of the quotes so that a hacker can't "get out" of the string and form her own MySQL statement.
Delan Azabani
+1  A: 

Remove the empty line before session_start():

?>

<?php
viriathus
Doesn't do anything because there is no empty line before session_start();Edit: Sorry, I get what you mean now :) Fixed that one up.
Kenny Bones
A: 
require("dbconnect.php")

should be

require("dbconnect.php");
r3zn1k
If I do that the same error message appear about the 'Cannot send session cache limiter' is shown again.
Kenny Bones
+1  A: 

The original error is due to a missing semicolon on the require line.

As others have said, you need to learn about sql injection and using placeholders. Get out of the habit of using submitted data without using placeholders or escaping first.

DGM
I don't even have a habit! I'm just fumbling around really, trying to learn all of this.
Kenny Bones
+1  A: 
<?php
//require_once("dbconnect.php");

$uploadstring = $_REQUEST['filename'];

$db_pswd = 'xxx-xxx-xxx';
$db_user = 'john_doe';
$db_table = 'my_table';

$con = mysql_connect( 'localhost' , $user , $pswd );
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

mysql_select_db( $db_table , $con );

mysql_query(" UPDATE brukere SET brukerBakgrunn = '".$uploadstring."'
WHERE brukerID = '".$_SESSION['id']."' ");

mysql_close($con);
?>

I think you need to use a fresh code! yours is compromised! ;-))

aSeptik
Well, I think what I need is to understand how this all work. I mean, the first part of your code is basically what I do as well, prior to logging on to the site. Then, after I've logged on, is the connection still open? Would I need to deliver connection properties again?
Kenny Bones