The 2009 CWE/SANS Top 25 Most Dangerous Programming Errors has been out for a while now. Maybe you've already eliminated all of these errors from your code. In case you haven't, this post will help you develop a checklist that you can use to eliminate these errors starting at the architecture level and moving through design, code, and testing. Before we get to the security aspect, first let's take a quick detour through the mechanics of a review checklist.
Why A Review Checklist is Essential
Most (all?) experts recommend the use of a checklist for performing design and code reviews. It's more effective to ask a set of questions about the item being reviewed than to simply stare at a bunch of code looking for problems. The checklist focuses your attention on aspects that are most important.
If you want to perform a broader review, develop two or three checklists with different foci and give the lists to separate reviewers. The defect lists from each reviewer will have less overlap than if you gave everyone the same list.What Your Review Checklist Should Look Like
The following advice is partly based on advice from "Software Inspection" by Gilb and Graham. A good checklist is a list of questions. For practical purposes, I'm strongly in favor of a checklist that does not exceed one side of a printed page — at normal font size. For me this means a checklist of about 15 items, 20 as an absolute maximum. Remember, we're focusing, which means looking harder for just a few types of items. If you're using online checklists, I'd extend this to mean that it should fit on the screen without scrolling. Either way, I don't think you should have more than 20 questions or so at the max. Fewer is probably better, more is too overwhelming.
Checklist items are yes/no questions. They should all be phrased such that a "no" answer means something is broken.When reviewing, proceed through the entire item under review (the entire code listing or design) with each question. Don't work linearly, this is a horrible way to review code. I prefer to review on paper, simply because I can make little notes and marks all over the paper as I go through it. For the first question, you might work mostly front-to-back. Then the second question might send you backwards through the code. After a couple of passes over the code you have some familiarity with it, so the third question might send you straight to a location that has potential problems.
How To Choose Questions for the Review Checklist
Since the whole point of the checklist (and reviews in general) is to find defects, the questions should focus on finding the highest value defects. Why waste valuable checklist real estate on a question like "Does the code conform to formatting conventions?" Duh. If your team has a formatting convention, and you look at a piece of code that doesn't conform, it's going to jump off the page and bite you in the nose. You don't need a checklist question for this!
Back to the specific problem at hand: security errors. SANS has done a great job of sifting through tons of security errors and boiling them down to the top 25. We could probably develop a 100 question review checklist from their Top 25 list, but it would be too big to be usable. So let's focus on the most important for our application.For this exercise, our application is a blog like Wordpress but much simpler: no plugins, no comments, single author. This is not a hosted application like Blogger, so we are less concerned about vulnerabilities like cross-site scripting, SQL injection, and OS command injection. We do need to make sure that only the author is allowed to post, so let's look at the "Porous Defenses" category.
- CWE-285: Improper Access Control (Authorization)
- CWE-327: Use of a Broken or Risky Cryptographic Algorithm
- CWE-259: Hard-Coded Password
- CWE-732: Insecure Permission Assignment for Critical Resource
- CWE-330: Use of Insufficiently Random Values
- CWE-250: Execution with Unnecessary Privileges
- CWE-602: Client-Side Enforcement of Server-Side Security
The entry for CWE-285: Improper Access Control lists the following for prevention and mitigation:
For web applications, make sure that the access control mechanism is enforced correctly at the server side on every page. Users should not be able to access any information that they are not authorized for by simply requesting direct access to that page. One way to do this is to ensure that all pages containing sensitive information are not cached, and that all such pages restrict access to requests that are accompanied by an active and authenticated session token associated with a user who has the required permissions to access that page.
Here are some straightforward checklist questions we can extract from that description:
- Does every administrative page enforce access control on the server side?
- Are unauthenticated (anonymous) users prevented from accessing administrative pages?
- Is caching disabled for administrative pages?
- Does every administrative page validate the session token properly?
Now you can go walk through your design and/or code to verify that you can say "yes" to each of these questions.
If you found this post helpful, subscribe to The Daily Build to get updates as they are posted. Thanks!"2009 CWE/SANS Top 25 Most Dangerous Programming Errors"