-snip-
[editline]4th November 2014[/editline]
Never mind, still got a lot to do
A few things:
- You don't escape input in most places, they are vulnerable to XSS. And remember to do this on the server side.
- I haven't checked, but do you require a captcha after a number of failed login attempts? Otherwise, someone could use an automated script to try and guess your users' passwords.
- Don't trust user input. Ever. Don't assume your forms are the only way people are going to submit input. See: -snip, OP knows what I mean-
I changed the value of the hidden "name" field in the topic creation form.
The same is possible for PMs and lots of other things.
- Again by editing the value, you can change your "sex" to anything you want in your profile. Also, while not a security question, don't assume everyone is male by default.
[editline]5th November 2014[/editline]
Oh. Umm. Sorry, couldn't really have helped without testing...
[QUOTE=DrTaxi;46412239]A few things:
- You don't escape input in most places, they are vulnerable to XSS. And remember to do this on the server side.
- I haven't checked, but do you require a captcha after a number of failed login attempts? Otherwise, someone could use an automated script to try and guess your users' passwords.
- Don't trust user input. Ever. Don't assume your forms are the only way people are going to submit input.
I changed the value of the hidden "name" field in the topic creation form.
The same is possible for PMs and lots of other things.
- Again by editing the value, you can change your "sex" to anything you want in your profile. Also, while not a security question, don't assume everyone is male by default.[/QUOTE]
Thanks for the help, I'll look into these issues. Also, can you remove the link from your comment? If it's that easy just to sign in as me, I don't want my site being posted here anymore.
[editline]4th November 2014[/editline]
Thanks! Glad you did that, now I can look into how to fix it so someone else doesn't do it and potentially screw up everything.
[QUOTE=GreenIguana;46412255]Also, can you remove the link from your comment?[/QUOTE]
Yeah, I figured, and removed it.
[QUOTE]If it's that easy just to sign in as me,[/QUOTE]
I didn't actually manage to sign in as you, you just ask the user's browser who they are in a few places, such as topic creation, instead of actually checking who they are logged in as (in session data).
You did it right in the password changing form, otherwise, I would have signed in as you for real :v:
One more thing. I couldn't (and can't) check this, but just in case you're storing user passwords in plain text in your database: Store salted hash values instead.
A hash is the output of a hash function, for example, sha256('123456') is 8d969eef6ecad3c29a3a629280e686cf0c3f5d5a86aff3ca12020c923adc6c92. This cannot be turned into "123456" again, so someone who only knows the hash value doesn't know it stands for '123456', so if someone steals your database, they can't read your users' passwords (and try using that information to log in to your site, or try the same password on their email account etc.).
However, if they guess that some of your users may have used '123456' as their password, they could try calculating sha256('123456') and see if any hash values in your database match.
This is where a salt comes in. A salt is a (preferably long) string unique to the user. For example, whenever someone registers, you could generate a 10-character random string, put it in the database, and store their password as $password.$salt.
Now instead of going through their list of passwords and calculating hash values for them once, they have to do that for every single user, making brute-force attacks unfeasible if your userbase is large enough.
You may already know this, in which case nevermind, but I figured it's worth mentioning.
[QUOTE=DrTaxi;46412382]Yeah, I figured, and removed it.
I didn't actually manage to sign in as you, you just ask the user's browser who they are in a few places, such as topic creation, instead of actually checking who they are logged in as (in session data).
You did it right in the password changing form, otherwise, I would have signed in as you for real :v:
One more thing. I couldn't (and can't) check this, but just in case you're storing user passwords in plain text in your database: Store salted hash values instead.
A hash is the output of a hash function, for example, sha256('123456') is 8d969eef6ecad3c29a3a629280e686cf0c3f5d5a86aff3ca12020c923adc6c92. This cannot be turned into "123456" again, so someone who only knows the hash value doesn't know it stands for '123456', so if someone steals your database, they can't read your users' passwords (and try using that information to log in to your site, or try the same password on their email account etc.).
However, if they guess that some of your users may have used '123456' as their password, they could try calculating sha256('123456') and see if any hash values in your database match.
This is where a salt comes in. A salt is a (preferably long) string unique to the user. For example, whenever someone registers, you could generate a 10-character random string, put it in the database, and store their password as $password.$salt.
Now instead of going through their list of passwords and calculating hash values for them once, they have to do that for every single user, making brute-force attacks unfeasible if your userbase is large enough.
You may already know this, in which case nevermind, but I figured it's worth mentioning.[/QUOTE]
Yeah, I reread your post and remembered how there's a hidden field for usernames. How would I be able to make this "safe" or able to pass without being changeable?
I use SHA-1 for hashing passwords, and all passwords in my database are hashed, not in plaintext, and have salts.
You mentioned earlier about escaping input, how would I go about doing this?
[QUOTE=GreenIguana;46412407]Yeah, I reread your post and remembered how there's a hidden field for usernames. How would I be able to make this "safe" or able to pass without being changeable?[/QUOTE]
You can't. Users can send you whatever they like.
On pages like your password changing form, you read the user's ID/name from $_SESSION, right? Session data is actually stored on the server, not the user. The user only gets a cookie, a variable stored in the user's browser, that holds a session ID. When a user requests a page with this cookie, PHP looks for the session ID in its session storage, and if a session with that ID exists, it provides a datastore corresponding to it as $_SESSION.
[QUOTE]You mentioned earlier about escaping input, how would I go about doing this?[/QUOTE]
Wrap your user input in htmlentities(). Either:
- Whenever you store user input text that is ever going to be written into any web page
Or:
- Whenever you output that user input text into any web page
You need to do the former if you want to manually insert HTML into e.g. a post in the database or allow certain users (administrators/you) to post HTML.
Note that, as of the time I was testing your site, the profile picture link and sex were user input text too. While you only display radio buttons for gender selection, an attacker could change the value of that radio button to whatever they want, and while the profile picture link isn't actually displayed as text on a profile, it's technically still text inserted into a webpage.
[QUOTE=GreenIguana;46412407]I use SHA-1 for hashing passwords, and all passwords in my database are hashed, not in plaintext, and have salts.[/QUOTE]
SHA-1 is getting deprecated very soon. You should use bcrypt or preferably scrypt.
[QUOTE=TrinityX;46415075]SHA-1 is getting deprecated very soon. You should use bcrypt or preferably scrypt.[/QUOTE]
I just figured it was better than md5, which was what was recommended on a tutorial I read up on a while back :P
Yeah, I will be looking into scrypt once I get these more immediate problems out of the way.
[editline]5th November 2014[/editline]
[QUOTE=DrTaxi;46412723]You can't. Users can send you whatever they like.
On pages like your password changing form, you read the user's ID/name from $_SESSION, right? Session data is actually stored on the server, not the user. The user only gets a cookie, a variable stored in the user's browser, that holds a session ID. When a user requests a page with this cookie, PHP looks for the session ID in its session storage, and if a session with that ID exists, it provides a datastore corresponding to it as $_SESSION.[/QUOTE]
Still a bit confused on this bit. In the hidden text field, it stores the session username, which can obviously be changed. Should I add like an if/else on the page the form redirects to to make sure that the form field is the same as the actual session again?
[QUOTE=GreenIguana;46418541]Still a bit confused on this bit. In the hidden text field, it stores the session username, which can obviously be changed. Should I add like an if/else on the page the form redirects to to make sure that the form field is the same as the actual session again?[/QUOTE]
No, you just take the hidden input field out and ignore it entirely.
[QUOTE=DrTaxi;46419298]No, you just take the hidden input field out and ignore it entirely.[/QUOTE]
And then define the username as the session user on the next page?
Yup, like that.
I'm honestly a bit surprised since that seems to be what you were already doing on all the pages that weren't vulnerable?
Use [URL="https://github.com/ircmaxell/password_compat"]password_compat[/URL]
[QUOTE=DrTaxi;46421057]Yup, like that.
I'm honestly a bit surprised since that seems to be what you were already doing on all the pages that weren't vulnerable?[/QUOTE]
Honestly, there is no rhythm or rhyme to any of my code, some pages I wrote months later than others, some I looked up tutorials for, and some I wrote based on nothing but what I knew.
Again, thanks for all the help. I appreciate it and I'll be spending the next few days incorporating all of the stuff you mentioned.
Sorry, you need to Log In to post a reply to this thread.
