Reviewing code for security issues

Security is consistently ranked as one of the top concerns for all of the engineering teams and freelancers who use PullRequest to help with code review; and PullRequest reviewers who frequently catch security issues are usually always rated very highly by the code authors who receive the review feedback.

Every PullRequest reviewer is expected to thoroughly examine the code in every review for security issues. This includes both the code that's part of a proposed change as well as any existing code in the review's files.

Common security-related issues caught by PullRequest reviewers include (but are not limited to) things like:

  • Known security vulnerabilities associated with a project's dependencies (and packages bundled with those dependencies).

  • Credentials hard-coded in source code.

  • Missing authorization or able to access information across accounts if code is abused.

  • Areas vulnerable to SQL injection attacks.

  • Ineffective, or missing, encryption of sensitive information.

  • Use of insecure cryptography.

  • Cryptography methods not abiding by framework best practices.

  • Improper credential/role management.

  • CRLF injection and XSS vulnerabilities. Ensuring user-supplied data is sanitized/neutralized appropriately.

  • File systems vulnerable to directory traversal.

  • Opportunities for unwanted information leakage.

  • Committing real user data in tests.

  • Improper integration of 3rd party tools.

Check out more common application security issues from OWASP here.

As with any feedback comment, links to external resources/documentation are highly encouraged.

Last updated