views:

46

answers:

5

Hi, I am creating a website which has user accounts. For each user account, the user can do stuff like update their personal details, write a blog etc. When the user wants to edit a blog, i have the following form (this is a simplied version).

<form action="goToThisPage.php" method="get">
    <input type="hidden" name="blogID" value="4" />
    <input type="text" name="blogTitle" value="" />
    <textarea name="blogContent"></textarea>
    <input type="submit" name="submit" value="Update Blog" />
</form>

Now the blogID as you can see is 4 for this user, so when they update the record, it'll update the blog table with ID 4. Now using firebug or other spoofing techniques, the user could change this ID to something like 8, and update record 8, which could be someone else's entry.

How do i prevent this? I've thought of two methods so far, wondering what you think is the best idea (or suggest another).

  1. Encode the ID with a random string, then decode the string once submitted, retrieving the correct ID.
  2. Leaave it as the numeric number and then check to make sure once updated, its their record by a database query.

I obviously want to limit the database queries, and by encoding the ID i believe is the better option. What do you guys think?

Thanks in advance

A: 

Doing anything on HTML level is a waste time. PHP simply shouldn't be allowing this, aka checking if the entry actually belongs to the user issuing the change.

Robus
when you say php shouldnt be allowing this? what do you mean?
phpNutt
Basically: Option two. Only you should be checking if its their entry before updating it.
Robus
+2  A: 

When building a web application, you'll be happiest if you assume the following:

  1. the client (that is, the browser) is not to be trusted. Assume any data sent to your server has been sent by a bad guy.

  2. the application is stateless. You can't assume that just because you intended request X to be preceded by request Y that they occurred that way.

Your option 2 is the better choice. If you require an authenticated user to update a blog post, and you require the user to be authorized to do so, then check those requirements in the code that updates blog posts. You probably will not have an issue with too many db queries, and if you do, you can deal with it when you do.

Ned Batchelder
A: 

There are many ways to approach this, some of them being:

  1. Hash (sha1 or similar) a secret string along with the id, on submit verify the hash. If it doesn't match, reject.

  2. Store all blogs user has access to in a session var, upon submit check if the submitted blog is in the session array, if not reject.

  3. Since you said you wanted to limit db queries, you could just append another where criterion to your update query. The actual query will of course depend on your db schema.

    UPDATE blogs SET ... snip ... WHERE BLOG id = FORM_SUBMITTED_ID AND blog.owner = CURRENT_USER

    This will make sure the update happens only if the user is the actual owner.

code_burgar
A: 

What if you used PHP session variables to store the blog entry?

phoffer
A: 

Ned Batchelder's answer contains some very important things to remember, I won't repeat them.

I will outline some more implementation details.

Assumptions:

  • The blog table has a column called ownerId, which holds the userId of whichever user owns/created the post.
  • You have some sort of user login system, and have a userId stored in session somewhere.

The simplest way to ensure user's only update their own posts is to simply check beforehand:

<?PHP
$blog = get_blog_by_id($_POST['blogId']);
if ($blog['ownerId'] != $_SESSION['userId']){
   die("You're a BAD MAN.  Cut it out!");
}
$blog['blogContent'] = $_POST['blogContent'];
$blog['blogTitle'] = $_POST['blogTitle'];

update_blog($blog); //escapes any strings, and runs an update.

If you really don't want pull the blog post from the database before updating it, for whatever reason, you could always do something like this:

<?PHP
$title = mysql_real_escape_string($_POST['blogTitle']);
$content = mysql_real_escape_string($_POST['blogContent']);
$id = mysql_real_escape_string($_POST['blogId']);
$userId = $_SESSION['userId'];

$sql = "UPDATE blog SET blogTitle='$title', blogContent='$content' WHERE blogId = '$id' AND ownerId = $user_id";

mysql_query($sql);

This saves you the initial lookup, but basically fails silently if the current user doesn't own the blog, since the WHERE condition will match zero records.

timdev