CFML coders... can you help a brother out?? Need some code looked at

[TechBoyJK]

If anybody here programs in CFML and knows enough to audit some code, can you help me? please?

I'm building a website that is going to require membership. I've designed a database structure (Microsoft SQL Server) that fits well for this application. I have the registration AND login process working. So from an end user perspective, everything functions just fine. Now that I have this in place and I have code that gets what I need done, done, I want to take a second look at it, audit it, get outside opinions, and rebuild it if necessary. I figure that even if it needs to be redone, I have enough code already that most of the thinking is done.

I have the process broken into 3 parts/documents.

step1.cfm - registration form and verification
step2.cfm - submission of form data to database
activation.cfm - activates account

Here is a link to a zip file containing the three files.

http://drop.io/pdkwkob

I've removed the lengthy display data/code, which is just the html that is used to display everything. You'll see a tag that says <div> content </div> where the display code would be.

With step1.cfm there is basically two parts to the document. 'Showform/NotShowForm'. The document starts out declaring all the needed variables and querying for the data it needs. There are some checks that determine if the form is complete or not. If it is not complete, or the user just pulled up the page and hasn't submitted anything, 'showform' will be 1, meaning the form to enter all the data needs to be displayed AND the supplied data (or lack of data) will not be submitted for verification. Once enough data is provided, showform is set to 0, in which this means that the user has provided sufficient information through the form. When show form is set to 0, it instead displays the data as the user submitted it, asking them to verify that it is ok.

Once they hit submit, the data is pass to step2.cfm, where it is inserted into the database, sends an email to the user (so they can click a link to verify the email address), and displays a 'thank you' message. At this point, all of the user data is setup in the database, and their account is flagged as 'new'. Once they get the email and click the link to verify, the account will be flagged as 'active' and they will be able to login and create a session.

 

[Cogman]

First off, I'm not a CMFL programmer, so take everything I say with a grain of salt .

Browsing through your code, and reading a bit on CMFL, Your code is somewhat clunky. (at least in step 1, I haven't looked through everthing). Rather then having several <CFIF> blocks, It would be better to do just one <CFSCRIPT> block and implement the function that you are after.

Other then the clunkyness of the code, You have a very simple database setup, one that is insecure. You should NEVER store a users raw password. If your security is compromised, your users shouldn't have to worry about their passwords being stolen. (The same could go for username, but people generally care less about that.)

In order to bulk up security, Apply a SHA-1, with some salt, or stronger hashing algorithm to password and store the hash instead of the password. After that, when checking if a password is correct, rehash the input password and compare hashs. Strong Hashs with salt are pretty hard to reverse (Without salt, they are a fair bit simpler to reverse, See Rainbow tables). Thus, if someone gets unauthorized access to the DB you don't have to worry, as much, that they will have all your customers (or employees) usernames and passwords.

When designing an interface with the outside world, security should always be the top concern.

 

[TechBoyJK]

First off, I'm not a CMFL programmer, so take everything I say with a grain of salt .

Browsing through your code, and reading a bit on CMFL, Your code is somewhat clunky. (at least in step 1, I haven't looked through everthing). Rather then having several <CFIF> blocks, It would be better to do just one <CFSCRIPT> block and implement the function that you are after.

Other then the clunkyness of the code, You have a very simple database setup, one that is insecure. You should NEVER store a users raw password. If your security is compromised, your users shouldn't have to worry about their passwords being stolen. (The same could go for username, but people generally care less about that.)

In order to bulk up security, Apply a SHA-1, with some salt, or stronger hashing algorithm to password and store the hash instead of the password. After that, when checking if a password is correct, rehash the input password and compare hashs. Strong Hashs with salt are pretty hard to reverse (Without salt, they are a fair bit simpler to reverse, See Rainbow tables). Thus, if someone gets unauthorized access to the DB you don't have to worry, as much, that they will have all your customers (or employees) usernames and passwords.

When designing an interface with the outside world, security should always be the top concern.

Thanks Cog! I thought one of the few things left in terms of security was going to be to hash the password. That was one of the last things on my list, but it was already there, because that's something that wasn't critical to make things 'work'. Thanks though. Points me in the right direction.