Secure Code Review: Code Review

Secure Code Review

Security-focused code review is a systematic process of analyzing source code to identify vulnerabilities, deviations from secure coding practices, and security policy violations before code is merged into production - it acts as the last line of defense before vulnerabilities are deployed into a real environment where they can be exploited by attackers. Although many organizations have established code review processes for code quality, performance, and maintainability, the security review component is often neglected or done superficially by reviewers without expertise in application security, resulting in the approval of code containing SQL injection, XSS, insecure deserialization, authentication bypasses, authorization flaws, and other critical vulnerabilities listed in the OWASP Top 10. Effective code reviews combine automated SAST (Static Application Security Testing) tools that rapidly scan code looking for known vulnerability patterns (SonarQube finding hardcoded credentials, Semgrep detecting insecure SQL concatenation, Checkmarx identifying missing input validation) with manual peer review conducted by developers with security awareness using standardized security checklists that cover specific OWASP categories (injection, broken authentication, sensitive data exposure, XXE, broken access control, security misconfiguration, XSS, insecure deserialization, insufficient logging, SSRF), integration with IDEs and CI/CD pipelines for immediate feedback during development (shift-left security), and support from security champions or the AppSec team for complex cases that require specialized expertise. The goal is not only to find bugs but also to educate developers about secure coding through constructive comments on PRs, gradually raising the security baseline of the entire team.

SAST: Static Analysis Tools

SAST tools analyze source code (or bytecode/binaries) without executing the application, using techniques such as data flow analysis, control flow analysis, taint analysis, and pattern matching to identify potential vulnerabilities - they are extremely efficient at detecting specific classes of bugs at scale (they can analyze millions of lines of code in minutes) but also generate false positives that need to be triaged. Enterprise tools such as Checkmarx, Veracode, and Fortify offer comprehensive coverage of languages and frameworks, IDE integration, dashboards for vulnerability tracking, and compliance support (customized reports for audits). Open-source alternatives include: SonarQube (detects code smells, bugs, and security hotspots in 30+ languages, integrable with Jenkins/GitLab/GitHub, has OWASP Top 10 rules), Semgrep (pattern-based analysis with custom rules in YAML, fast, low false positive rate, used by Snowflake and Dropbox), Bandit for Python (specific to security issues), Brakeman for Ruby on Rails, SpotBugs for Java, ESLint security plugins for JavaScript. Integrate SAST into the CI/CD pipeline: configure automatic scanning on every pull request, block the merge if high/critical vulnerabilities are found, but allow developers to mark findings as false positives or accepted risks with justification so as not to block velocity unnecessarily. Configure appropriate severity thresholds - blocking on all warnings will frustrate developers and create security theater, block only on genuine criticals and highs. Keep rulesets updated and customize them for your stack - disable rules for frameworks you don't use, add custom rules for patterns specific to your organization (for example, a rule that detects the use of your company's deprecated APIs).

OWASP Security Checklists

Security checklists provide a standardized structure for reviewers to verify critical security aspects during manual code review - they ensure consistency among reviewers and reduce the chance of overlooking common vulnerabilities. Use the OWASP Code Review Guide and ASVS (Application Security Verification Standard) as a basis for creating checklists customized to your context. Essential categories to include: INPUT VALIDATION - is all user input (query params, body, headers, cookies) validated and sanitized? Is whitelisting of allowed characters implemented? Are length limits enforced? AUTHENTICATION - are passwords hashed with bcrypt/Argon2 (not MD5/SHA1)? Are session tokens generated cryptographically secure? Does logout invalidate the session server-side? Is MFA implemented where appropriate? AUTHORIZATION - do permission checks happen server-side (not just frontend)? Do access control decisions use the user identity from the session (not params that can be manipulated)? Are direct object references protected with authorization checks? CRYPTOGRAPHY - is sensitive data encrypted at rest and in transit? Are cryptographic keys stored securely (not hardcoded)? Are strong algorithms used (AES-256, RSA-2048+, not DES/RC4)? SQL INJECTION - do queries use prepared statements or ORMs? Is string concatenation for building SQL absent? Is user input never interpolated directly into queries? XSS - is output escaped based on context (HTML entity encoding, JavaScript encoding, URL encoding)? Are Content Security Policy headers configured? SENSITIVE DATA - are secrets/tokens not logged or exposed in error messages? Is sensitive data not returned in responses unnecessarily? ERROR HANDLING - are stack traces and detailed error messages not exposed in production? Are errors logged server-side for debugging while generic messages are shown to the user? Create a specific checklist for each type of change: new API endpoints have a checklist focused on input validation and authorization, changes to the authentication flow have a checklist for credential storage and session management, frontend changes have a checklist for XSS and CSRF.

Manual Peer Review by Developers

Although SAST tools are powerful, manual peer review by developers remains irreplaceable for detecting logic flaws, business logic vulnerabilities, and context-specific issues that tools cannot identify - for example, an authorization bypass where the code is technically correct but business logic allows improper access, race conditions in concurrent code, timing attacks in comparisons of sensitive strings, or side-channel leakage of information through different error messages. Establish a formal security-focused code review process: every PR must be reviewed by at least one developer besides the author (ideally a security champion or someone with OWASP/security training), the reviewer should run the code locally if possible to understand real behavior (not just read the diff), use debugging tools to validate authentication/authorization flows, test with malicious inputs (SQL injection payloads, XSS vectors, path traversal attempts) to verify that validations work, verify that unit tests include security test cases, and leave constructive comments explaining not only the problem but how to fix it and why it matters. Avoid "rubber stamp reviews" where the reviewer simply approves without real analysis - establish the expectation of a quality gate where security review takes appropriate time. For large changes (5000+ lines), consider doing the review in stages or a live pair programming session where the author explains the code and the reviewer questions security decisions. Recognize and reward reviewers who find vulnerabilities - build a culture where security findings are celebrated as saving the company from a breach rather than criticizing the developer who wrote insecure code. Maintain a knowledge base of vulnerabilities found in reviews with examples of vulnerable and fixed code, used for training new developers.

Secure Coding Standards and Frameworks

Establish and enforce secure coding standards that developers must follow - it cannot be just a PDF document that no one reads, but rather standards implemented through code snippets, internal libraries, framework configurations, linters, and automated checks that make "the secure way" also "the easy way". Examples of standards: for input validation, provide a centralized library with pre-built validators for email, phone, CPF, credit card, etc. that developers simply import and use instead of writing their own bug-ridden regex; for SQL queries, enforce the use of an ORM (Hibernate, Entity Framework, Sequelize) or query builders that automatically use parameterized queries; for authentication, provide an internal SDK that implements OAuth2/OIDC correctly instead of each team creating its own implementation; for cryptography, provide a crypto library wrapper that exposes only approved algorithms (AES-256-GCM, ChaCha20-Poly1305) and hides the complexity of key management; for logging, provide a logger that automatically redacts sensitive data (passwords, tokens, credit cards) before writing logs. Document prohibited anti-patterns with examples of vulnerable code: string concatenation for SQL queries - PROHIBITED, always use PreparedStatement; eval() of user input - NEVER; storage of passwords in plaintext or MD5 - use bcrypt with salt; comparison of sensitive strings with == - use constant-time comparison; Random() for security tokens - use SecureRandom/crypto.randomBytes. Configure linters (ESLint security plugins, Pylint, RuboCop security cops) to detect these anti-patterns automatically in IDEs and CI. Conduct regular secure coding trainings (quarterly) with hands-on exercises where developers identify and fix vulnerabilities in sample code, use gamification with leaderboards for engagement.

Integration with CI/CD and DevSecOps

Integrate security code review into the CI/CD pipeline to automate as much as possible and give developers rapid feedback - waiting for manual security review from the AppSec team for every PR creates a bottleneck and delays; shift security left by putting the tools in the hands of developers. Example pipeline: developer creates PR → GitHub Actions trigger → SAST scan runs (SonarQube, Semgrep) → dependency check runs (OWASP Dependency-Check, Snyk, npm audit) looking for vulnerabilities in libraries → secret scanning runs (git-secrets, TruffleHog, GitHub Advanced Security) looking for committed credentials → results are posted as comments on the PR with links to remediations → if critical/high vulnerabilities are found, the status check fails and the merge is blocked until the developer fixes it → if all checks pass, the PR goes to manual peer review → after approval, the merge happens → the deployment pipeline runs DAST (Dynamic Application Security Testing) in the staging environment → if DAST passes, deploy to production. Configure SAST tools to "fail fast" - run on each commit locally (pre-commit hooks) to catch issues even before push, not just in CI. Use quality gates in SonarQube that define thresholds: code coverage must be greater than 80%, security hotspots must be 0, critical bugs must be 0, vulnerabilities must be 0. Important: balance security with developer experience - if the security pipeline takes 45 minutes to run and frequently blocks due to false positives, developers will look for workarounds to bypass it; optimize for fast runs (cache dependencies, run checks in parallel) and tune rules to minimize false positives. Build dashboards with security metrics: number of vulnerabilities found per team/sprint, mean time to remediate, percentage of code covered by security tests, false positive rate of tools - use these metrics for continuous improvement of the process.