tags:

views:

117

answers:

8

In my profile.php script, I have a flag function that allows users to flag that user.

If they flag a user, it sends data (user_id, reason, etc) to a file called flag.php which does all the banning and stuff. The data is sent to flag.php through

header("Location: flag.php?user_id=___&reason=___")

Then in flag.php, after it does all the banning, it redirects the user back to the profile through another header. The user never sees the flag.php.

Is my flag.php safe? because they never see the script?

EDIT You should never assume it's safe for GETs...Going to either do a banuser function or send it through sessions.

+7  A: 

Nope. The URL will be recorded in the browser history, and the user you just banned will be able to ban any user they want to ban because of the user_id parameter. I suggest you create a function ban_user($user_id,$reason) instead.

waiwai933
mm I knew it wouldn't be safe. What about after the user submits the data, it gets stored in a session. Then in my flag.php, if say $_SESSION['banneduser_id'] is set, it would do the function.
ggfan
A: 

304-redirect have to go through the client, so the user could monitor the HTTP messages and learn how to control flag.php.

How about using $_SESSION, if it must live in a new page?

KennyTM
A: 

It's not safe. If one knows the location (which is easy by checking headers/requests with a tool), one could just enter flag.php?user_id=1&reason=bye in the URL to get the desired user banned. One could even write a script which does this in a loop from user id 1 to 100000.

You'll need to do this in a standalone script which you include in the main PHP script, not by a HTTP request. Ensure that the user authentication is robust as well.

BalusC
A: 

Security by obscurity (as in not seeing the script) is a bad thing to do. If it's a popular page, people WILL look for ways around the system, and smart ones will try messing with input!

What if they decide to rewrite some URL-variables by putting in "HACK ATTEMPT!&user_id=[root_id]"

PHP might just take the first "user_id" it finds, but you can see where this is going.

LukeN
A: 

Your code should check to see if:

  1. The user doing the flagging is logged in
  2. The user doing the flagging has visited the flag.php page directly
  3. The user doing the flagging has flagged nn people in the last few minutes

Is there some reason why you can't just POST the data to flag.php, without using a redirect, then redirect them back to where they were (if indeed, they were somewhere and not just abusing flag.php) ?

I also suggest putting a captcha in the process, if you detect strange behavior.

Anyway, no, its not safe. Its just asking for abuse.

Tim Post
A: 

It is totally unsafe; any user can be made to ban any other user against his will. Look into CSRF attacks and defenses. And drop the redirect thing, it is utterly pointless. I'm not even sure what you wanted to achieve with it. If the user sends a request to wherever the flagging link points, instead of forging a direct request, are you any better off? I don't think so.

Tgr
A: 

No. Your script is not safe for the reasons mentioned in the other answers.

A solution, though - and the one I use in certain cases, is to also require a unique ID as a parameter. For example:

flag.php?user_id=_______&reason=______&special_id=36a678dabc6d78a6db

The special_id would only be valid for banning the user with ID user_id.

This ID would only be valid for one use - and it would only be valid for the specified id.

George Edison
A: 

If I put a function at the beginning that makes sure that the logged in person is an admin, and if it is, it would do the script. But if the logged in person isn't an admin, it would redirect to the homepage.

Would that be make it safe to use GETs?

ggfan
No it is not. <img src="http://yourdomain.com/flag.php?user_id=1"> on a completely different server could trigger it if you are logged in, for example.
Macha
Alright, I give up on using GETS. sessions and functions are the way to go
ggfan