Return to Revisions

I think it is imperative that the security of your code be addressed in detail, as I can see quite a few things that need desperately to be changed. I'll see what I can do about going through each one, and for now they are just in an order of which I've noticed them!

1. The first security issue I see is the size and scope of your class. This may sound irrelevant, but the complexity of your code will indirectly affect how secure your code is. This isn't the case each time, but as a beginner, I suggest the SOLID principles.

   Keeping your classes small, modular, and encompassing only a single responsibility will help keep your mind focused and you will find it much easier to write secure code. If you have ten functions, only somewhat related, and each has a different security protocol, it is almost guaranteed you will get confused. Allow yourself a simple, efficient, system to effectively and properly lock down your application. I see a lot of repetition of md5() and lots of session setting, un-setting, and validating. KISS!

2. Again, I see md5 used in several instances. The first instance I see is md5($this->auth_key). What's $auth_key though? Well it looks like it's pointing the the global constant SECURE_AUTH_KEY. So, based on this information, I can assume you don't want thing variable being compromised. Yet, it alreayd is. It's in a cookie, which does not make any sense at all, but we'll get back to that.

   So, that key is really not a key at all. A key is for encryption, and it determines the ciphertext. What you have is a password. It's hard to tell why you have used md5, becuase having a hash here did not add any security to your application. The problem is that every user has the same hashed session value, which can easily be duplicated, and then bam! your log in is comprimised! Sorry :/

What can you do? Unfortunately, what you have is poorly structured, and so anything I suggest will just confuse everyone! Even trying to implement either bcrypt, scrypt, or PBKDF2 would simply make things over the top complicated and I think it would be unwise to dive into that at your level of expertise. I will suggest you don't do this, and instead, take a look at this, this, and this. I highly recommend all three. 3. What you store in cookies and sessions should be assumed to be compromised. Having an SSL connection could protect you from man-in-the-middle attacks, but even then you are not guaranteed perfect security. Here is a recommended read about session security. 4. You currently are susceptible to SQL injections. I recommend combining Defense Option 1 and 2 from OWASP to protect you. I wrote a good summary of SQL injection protection with a focus on PHP here, which I also suggest you read.

I'm sure there's more that could be determined on a security standpoint, but that should get you started in a sucess bound direction! I wouldn't provide a link to you if I didn't think it would benefit, so I highly suggest you understand the contents behind each one of the links I've supplied.