(02-11-2015, 02:50 AM)Narf Wrote: I took a quick look at your addon. Here are a few flaws that I've noticed:
- You're putting a limit on passwords ... Don't do that. Checking if a password is strong enough is fine (and I encourage you to continue doing that), but don't artificially limit its maximum length.
- You've got xss_clean applied to all input fields. XSS filtering should be applied on output, not input. This is especially bad for passwords, since it can strip special characters from them and silently make them weaker than the user intended.
- You're using GET, POST input data without any kind of validation (both in controllers and models). You've got at least two SQL injection vulnerabilities triggered by Usermodel::get_user_permissions() alone. Please validate all input data, even if you don't pass it to a database.
- You're depending on a password hashing library that was last updated in 2006. A quick glance at it reveals that at least it's get_random_bytes() function is insecurely implemented for any system that you wouldn't have access to /dev/urandom on ... I'm sure I'll find more issues with it if I take a closer look. Just use ircmaxell/password_compat, it's the only proven solution for password hashing in PHP (CI3 already has it).
Now, somebody will probably jump on me for saying this and accuse me of being rude and whatnot, but don't take the following as a personal insult ... it's not, it's just common sense when it comes to security. Please take this as a friendly advice:
You're obviously not qualified enough to maintain a security-sensitive library - it just takes a lot of skill and knowledge that, unfortunately, few people in the CI community have. Your addon is attractive because you've made it fancy and I guess easy to use, but it exposes its users to critical security issues. Please redirect your efforts to improving an existing, proven solution instead ... I know there are at least two of them that are CI-specific, if that's your criteria.
Which Auth libraries are proven and CI specific. In the past I have used DX_Auth and was in the process of updating the old code to work with CI3. I ask because I like DX_Auth but don't want to waist my time if it is not worth it. Thanks!!!!
