Latest revision as of 19:09, 5 December 2015

Jim Manico 12/5: This is very good feedback. I suggest diving into the main doc and make the edits live, we have history and rollback here in case there is an issue. But I think your comments are astute and will enhance this cheatsheet.

Current Comments

I’ve been reviewing this document in hopes of getting it out of Draft status by the end of the year.

My comments fall into three overall categories: 1) Overall structure of the document 2) Excluding some items that seem to me beyond the scope of security and venture into business or functional-spec testing (important topics, but not security-focused per se) 3) Various other details.

Overall structure of the document

I like the checklist-oriented approach of this document, and I like most of the overall category breakdown. But I have a few comments:

• Right now, the contents list three sections: [1] Introduction, [2] Purpose, and [3] The Checklist. However, this checklist piece includes credits/authors etc. I believe we need to move these to a new fourth section: “[4] Appendices”
• I also believe that readers would be well served by seeing some of these categories split further into subcategories.

So right now the overall order of the categories is this:

1 Introduction

2 Purpose

3 The Checklist

-- -- 3.1 Information Gathering

-- -- 3.2 Configuration Management

-- -- 3.3 Secure Transmission

-- -- 3.4 Authentication

-- -- 3.5 Session Management

-- -- 3.6 Authorization

-- -- 3.7 Data Validation

-- -- 3.8 Denial of Service

-- -- 3.9 Business Logic

-- -- 3.10 Cryptography

-- -- 3.11 Risky Functionality - File Uploads

-- -- 3.12 Risky Functionality - Card Payment

-- -- 3.13 Web Service Testing

-- -- 3.14 HTML 5

-- -- 3.15 Error Handling

-- -- 3.16 Other Formats

-- -- 3.17 Authors and contributors

-- -- 3.18 Related articles

-- -- 3.19 Other Cheatsheets

I’d suggest a change to the following (Note that some of my comments further below actually would modify this further still):

1 Introduction

2 Purpose

3 The Checklist

-- -- 3.1 Information Gathering

-- -- -- -- Rendered Site Review

-- -- -- -- Development Review

-- -- -- -- Hosting and Platform Review

-- -- 3.2 Configuration Management

-- -- 3.3 Secure Transmission

-- -- 3.4 Authentication

-- -- -- -- Application Password Functionality

-- -- -- -- Additional Authentication Functionality

-- -- 3.5 Authorization

-- -- 3.6 Cryptography

-- -- 3.7 Session Management

-- -- 3.8 Data Validation

-- -- 3.9 Denial of Service

-- -- 3.10 Business Logic

-- -- 3.11 Specific Risky Functionality

-- -- -- -- File Uploads

-- -- -- -- Payments

-- -- 3.12 Web Service Testing

-- -- 3.13 HTML 5

-- -- 3.14 Error Handling

4 Appendices

-- -- 4.1 Other Formats

-- -- 4.2 Authors and Contributors

-- -- 4.3 Related Articles

-- -- 4.4 Other cheatsheets

I would also remove HTML5 and distribute its contents under other categories. (Place WebStorage Injection under Data Validation, etc.)

Excluding some items that seem to me beyond the scope of security and venture into business or functional-spec testing (important topics, but not security-focused per se)

There are a few items in here that seem to me to go beyond the bounds of security. I recognize that part of the value of a checklist is to be comprehensive but I also think this level of comprehensiveness needs to be balanced by focus (in our case, a security focus):

• Under “Configuration Management” we have listed “Test for non-production data in live environment, and vice-versa.” This seems more of a business-testing issue to me (are the data accurate) rather than security. While this could have a security impact – such as incorrect passwords or exposure of data-- I think that other points of the cheatsheet would in fact catch these issues.(I really worry that this bloats the checklist and opens a bit of a pandora’s box – for instance I could make an argument, along the same lines, that “badly labelled textboxes” could also cause a security issue – if “last name” is marked “password” a user could store their password in cleartext… but presumably correct labelling is a business-testing issue rather than a specific security-testing issue)

• The entire “Business Logic” category also seems problematic to me in this context. Several of these items are not security focused while others could belong in other categories (“Test for integrity of data” seems to belong in Data Validation to me, while “Test Upload Of Unexpected File Types” belongs under “Risky Functionality: File Uploads”). I would remove this category and relocate relevant bullet points to other areas.

Various other details

• Under “Purpose” the sentence “All feedback or offers of help will be appreciated - and if you have specific chances you think should be made, just get stuck in.” sounds awkward to me: I suggest “All feedback or offers of help will be appreciated - and if you have specific changes you think should be made, please log in and make suggestions.”
• I’d include a point under authentication regarding Oauth2 and/or SAML: “Test oAuth2 and/or SAML approaches, and ensure that most appropriate methods and scopes are used.”
• I’d include a point on regarding network rights on the host (what user account is the app run under, and are the rights appropriate - for example, excluding Execute rights for the user an app is run under is yet another safeguard against malicious file upload)

- VM