#1
  1. A Change of Season
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Mar 2004
    Location
    Nobbies beach, Gold Coast. It's beautiful.
    Posts
    2,574
    Rep Power
    171

    Is this proper script for stopping csrf


    Hi I wonder if this link is correct and reliable information.

    Thanks
  2. #2
  3. Come play with me!
    Devshed Supreme Being (6500+ posts)

    Join Date
    Mar 2007
    Location
    Washington, USA
    Posts
    13,749
    Rep Power
    9397
    As long as you're using two separate scripts for the form (one to show it, another to receive the data) then yes that method is safe against CRSF.
  4. #3
  5. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,920
    Rep Power
    1045
    Hi,

    no, it's not. This code is dangerous and low-quality. The guy who wrote (or rather copypasted) it doesn't know what he's doing. Do not use it.

    • The code uses the GET method for a data-altering action. This is a security risk, because it makes it very easy to forge requests. All you need to do is have the victim visit a page under your control. A simple img element with the target URL as the source will then automatically trigger a request. Using GET in this case is also semantically incorrect: GET is supposed to get a resource, that's why it's named "get".
    • The CSRF check doesn't validate that there even is a token in the session. If there's isn't (maybe right after the login), then the attacker can simply leave out the URL parameter and will gracefully pass the test.
    • Passing the token via the URL is a huge security risk given that many people carelessly share links, and it's an invitation for every social engineer. The average user has never heard of "CSRF" in their whole life, so it shouldn't be too hard to ask them for a link with a valid token in it.
    • None of the two "random" number generators are suitable for security applications. The manual specifically says that. This is #6 of the security sins, by the way.
    • Applying md5() to uniqid() makes no sense whatsoever and introduces the risk of collisions.

    You need to check your resources. Just look at the site:
    • The article is 6 years old.
    • The last article is from 2010, after that, there's only a maintenance note from last year. So the site has probably long been abandoned.
    • Pick any article, and you'll find crap spaghetti code straight from the 90s with <font> and bgcolor and whatnot.
    • There's not even a syntax highlighter.

    Now, honestly, do you expect to get professional security advice from this site? Do you think that's where the knowledgeable people hang out? I doubt it. To me, this looks like one of the countless "code for free" websites spreading crap code they copypasted from other "code for free" websites.

    I understand that they're somehow affiliated with Dev Shed, so I hope I'm not gonna get banned for this. But that doesn't change the fact that the code is plain crap.

    To generate a secure random number, use openssl_random_pseudo_bytes() or mcrypt_create_iv() as explained in the security sins.
    Last edited by Jacques1; July 15th, 2013 at 11:31 AM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  6. #4
  7. Come play with me!
    Devshed Supreme Being (6500+ posts)

    Join Date
    Mar 2007
    Location
    Washington, USA
    Posts
    13,749
    Rep Power
    9397

    as much as I hate to admit it, he's right


    Originally Posted by Jacques1
    The code uses the GET method for a data-altering action. This is a security risk, because it makes it very easy to forge requests. All you need to do is have the victim visit a page under your control. A simple img element with the target URL as the source will then automatically trigger a request. Using GET in this case is also semantically incorrect: GET is supposed to get a resource, that's why it's named "get".
    True. If the script additionally checked that the CSRF token was set in the first place then it would be safe and the request would be blocked by the token: either it wasn't set (first visit to that page) and it stops, or the token doesn't match and it stops.

    Originally Posted by Jacques1
    The CSRF check doesn't validate that there even is a token in the session. If there's isn't (maybe right after the login), then the attacker can simply leave out the URL parameter and will gracefully pass the test.
    That's the hole.

    Originally Posted by Jacques1
    Passing the token via the URL is a huge security risk given that many people carelessly share links, and it's an invitation for every social engineer. The average user has never heard of "CSRF" in their whole life, so it shouldn't be too hard to ask them for a link with a valid token in it.
    Goes back to the form using GET when it shouldn't. If someone can convince a user to hand over sensitive information, whether they know it is or not, then you're screwed anyways - being in the URL just makes it easier. The second script can help with this by regenerating/invaliding the token, thus the link shared would no longer work.

    Originally Posted by Jacques1
    None of the two "random" number generators are suitable for security applications. The manual specifically says that. This is #6 of the security sins, by the way.
    The attacker has no idea where or when the link will be triggered. They cannot anticipate the hash.

    Originally Posted by Jacques1
    Applying md5() to uniqid() makes no sense whatsoever and introduces the risk of collisions.
    Much more unlikely given how strictly the input is formed but fine.

    Originally Posted by Jacques1
    I understand that they're somehow affiliated with Dev Shed, so I hope I'm not gonna get banned for this.
    Oh please. If we were going to ban you it wouldn't be over that.
  8. #5
  9. No Profile Picture
    Dazed&Confused
    Devshed Novice (500 - 999 posts)

    Join Date
    Jun 2002
    Location
    Tempe, AZ
    Posts
    501
    Rep Power
    127
    Originally Posted by requinix
    Oh please. If we were going to ban you it wouldn't be over that.
    It sounds like you have other reasons in mind.
  10. #6
  11. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,920
    Rep Power
    1045
    Originally Posted by requinix
    The attacker has no idea where or when the link will be triggered. They cannot anticipate the hash.
    They can even control the exact point of time when the hash is generated, simply by forging the initial request as well: First they have the user GET the form page, which generates a new token at a known point of time and stores it in the session. And then they start bruteforcing the token.

    Neither rand() nor uniqid() nor combinations of the two are even remotely suitable for any security purpose. It's just that they somehow made it into the PHP folklore, and now nobody can get them out of there, no matter how many big yellow warnings the PHP developers put into the manual.

    This script is a failure on pretty much every level. It cannot be fixed except by throwing it away altogether.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  12. #7
  13. Come play with me!
    Devshed Supreme Being (6500+ posts)

    Join Date
    Mar 2007
    Location
    Washington, USA
    Posts
    13,749
    Rep Power
    9397
    Originally Posted by Jacques1
    They can even control the exact point of time when the hash is generated, simply by forging the initial request as well: First they have the user GET the form page, which generates a new token at a known point of time and stores it in the session. And then they start bruteforcing the token.
    The attacker would have to:
    1. Know when the user clicked the link, using that as a starting point to brute-force times. Say they can figure it out to within a second or so, then say another second for lag.
    2. Know that the MD5 hash is generated with a uniqid(time(),true). Guessable but difficult to verify.
    3. Brute-force the hash until they find the (roughly) seven a-z0-9 + ten 0-9 pseudo-random characters in the input*
    4. Trick the user into visiting many, many times until it works

    If the code reset the token upon form submission then that changes the requirements: you wouldn't have to pre-seed the token by making them visit the original form again, but you'd still have to guess a new token for each attempt. And each attempt is independent.

    * uniqid(time(), true) results in strings like
    Code:
    TTTTTTTTttHHHHHHHhhhhhhh.nnnnnnnn
    
    T = part of the time that is easily guessable (0-9)
    t = part of the time that is still guessable but varies over a few seconds (0-9)
    H = part of the uniqueness uniqid() adds but is easily guessable as it varies with time (like T) (0-9a-f)
    h = part of the uniqueness uniqid() adds but is hard to guess as it varies wildly with time (better than t) (0-9a-f)
    n = microseconds (0-9)
    Even rounding down, say 1t + 5h + 8n, that's still astronomical at 11,258,999,068,426,240 possibilities.
    Last edited by requinix; July 15th, 2013 at 06:11 PM.

IMN logo majestic logo threadwatch logo seochat tools logo