Checklists are a highly recommended way to find the things you forget to do and are useful for the reviewer. A checklist is the single best way to combat the problem, as it reminds the reviewer to take the time to look for something that might be missing. A checklist will remind reviewers to confirm that all errors are handled, that function argument is tested for invalid values, and that unit tests have been created.
In this article, you will learn what is a code review checklist of makeen transform.
General guideline
- Code should be readable and meaningful with proper comments.
- Functions and variable names should be meaningful.
- See the code running with the developer.
- Look for any cases that might not have been handled.
- Compare with the use case and user story to see any requirement is missed out.
- Follow SOLID principles as much as possible.
- Avoid repetition of code.
C# general code review checklist
- StringBuilder is for more than 5 string operations only else normal strings are used.
- Avoid using var and dynamic if the variable type is obvious.
- All code covered in a try block, there should be no unnecessary exceptions thrown, no empty catch blocks, no try inside catch. Do not rethrow an exception.
- Dispose method is implemented along with using statement.
- Use && instead of & and || instead of |, a condition that is most likely to pass/fail should be right at the beginning.
- Avoid using too much nested ternary operators.
- Avoid functions with one or a few lines of code.
- Use a struct instead of classes for very simple classes not requiring boxing and unboxing.
- Use string compares instead of converting the case to lower and upper and then using ==.
- Try to initialize variables along with the declaration.
- Try to use the “@” prefix for string literals instead of escaped strings.
- Do not compare strings to String. Empty or “" compare by using String.Length == 0.
- Try to avoid repetition of if checks.
- Try to minimize loops.
- Avoid declaring methods with more than 5 parameters. Consider refactoring this code.
- Never define a Finalize() method as a finalizer. Instead, use the C# destructor syntax.
- No hardcoded parameters, all parameters should be either in the config file or DB.
- Comments are in Doxygen format, multiline /** and */, inline /**< and */.
- Check 'null' wherever applicable.
- Access specifiers are properly used.
General secure code review checklist
Authentication:
• Ensure all internal and external connections (user and entity) go through an appropriate and adequate form of authentication. Be assured that this control cannot be bypassed.
• Ensure all pages enforce the requirement for authentication.
• Ensure that whenever authentication credentials or any other sensitive information is passed, only accept the information via the HTTP “POST” method and will not accept it via the HTTP “GET” method.
• Any page deemed by the business or the development team as being outside the scope of authentication should be reviewed to assess any possibility of the security breach.
• Ensure that authentication credentials do not traverse the wire in clear text form.
• Ensure development/debug backdoors are not present in the production code.
Authorization:
• Ensure that there are authorization mechanisms in place.
• Ensure that the application has clearly defined the user types and the rights of said users.
• Ensure there is a least privilege stance in operation.
• Ensure that the Authorization mechanisms work properly, fail securely, and cannot be circumvented.
• Ensure that authorization is checked on every request.
• Ensure development/debug backdoors are not present in the production code.
Cookie Management:
• Ensure that sensitive information is not comprised.
• Ensure that unauthorized activities cannot take place via cookie manipulation.
• Ensure that proper encryption is in use.
• Ensure a secure flag is set to prevent accidental transmission over “the wire” in a non-secure manner.
• Determine if all state transitions in the application code properly check for the cookies and enforce their use.
• Ensure the session data is being validated.
• Ensure cookie contains as little private information as possible.
• Ensure the entire cookie should be encrypted if sensitive data is persisted in the cookie.
• Define all cookies being used by the application, their name, and why they are needed.
Data/Input Validation:
• Ensure that a DV mechanism is present.
• Ensure all input that can (and will) be modified by a malicious user such as HTTP headers, input fields, hidden fields, drop-down lists & other web components are properly validated.
• Ensure that the proper length checks on all input exist.
• Ensure that all fields, cookies, HTTP headers/bodies & form fields are validated.
• Ensure that the data is well-formed and contains only known good chars if possible.
• Ensure that the data validation occurs on the server-side.
• Examine where data validation occurs and if a centralized model or decentralized model is used.
• Ensure there are no backdoors in the data validation model.
• Golden Rule: All external input, no matter what it is, is examined and validated.
Error Handling/Information leakage:
• Ensure that all method/function calls that return a value have proper error handling and return value checking.
• Ensure that exceptions and error conditions are properly handled.
• Ensure that no system errors can be returned to the user.
• Ensure that the application fails securely.
• Ensure resources are released if an error occurs.
Logging/Auditing:
• Ensure that no sensitive information is logged in the event of an error.
• Ensure the payload being logged is of a defined maximum length and that the logging mechanism enforces that length.
• Ensure no sensitive data can be logged; E.g. cookies, HTTP “GET” method, authentication credentials.
• Examine if the application will audit the actions being taken by the application on behalf of the client (particularly data manipulation/Create, Update, Delete (CUD) operations). Next: Code Review Process of makeen transform.
• Ensure successful and unsuccessful authentication is logged.
• Ensure application errors are logged.
• Examine the application for debug logging with the view to logging of sensitive data.
Cryptography:
• Ensure no sensitive data is transmitted in the clear, internally or externally.
• Ensure the application is implementing known good cryptographic methods.
Secure Code Environment:
• Examine the file structure. Are any components that should not be directly accessible available to the user?
• Examine all memory allocations/de-allocations.
• Examine the application for dynamic SQL and determine if it is vulnerable to injection.
• Examine the application for “main()” executable functions and debug harnesses/backdoors
• Search for commented out code commented out test code, which may contain sensitive information.
• Ensure all logical decisions have a default clause.
• Ensure no development environment kit is contained in the build directories.
• Search for any calls to the underlying operating system or file open calls and examine the error possibilities.
Session management:
• Examine how and when a session is created for a user, unauthenticated, and authenticated.
• Examine the session ID and verify if it is complex enough to fulfill requirements regarding strength.
• Examine how sessions are stored: e.g. in a database, in memory, etc.
• Examine how the application tracks sessions.
• Determine the actions the application takes if an invalid session ID occurs.
• Examine session invalidation.
• Determine how multithreaded/multi-user session management is performed.
• Determine the session HTTP inactivity timeout.
• Determine how the log-out functionality functions.
Need more help? Contact us at support@makeen.io.