secure-code-review

Conduct comprehensive security code reviews using OWASP Top 10, SAST/DAST patterns, and Hack23 ISMS secure development policy

Hack23 236 56 Updated 6mo ago

GitHub

Install

npx skillscat add hack23/cia/secure-code-review

Install via the SkillsCat registry.

SKILL.md

Secure Code Review Skill

Purpose

This skill provides strategic guidance for conducting thorough security code reviews that identify vulnerabilities before they reach production. It implements defense-in-depth principles aligned with OWASP Top 10, SANS Top 25, and Hack23 ISMS Secure Development Policy.

When to Use This Skill

Apply this skill when:

✅ Reviewing pull requests before merge
✅ Conducting periodic security audits of existing code
✅ Implementing new features that handle sensitive data
✅ Integrating third-party libraries or APIs
✅ Refactoring authentication/authorization logic
✅ Before major releases or production deployments
✅ After security incidents or vulnerability disclosures

Do NOT use for:

❌ General code style reviews (use code-quality-checks skill)
❌ Performance optimization (different concern)
❌ Business logic validation (functional testing)

Decision Tree

START: Code Review Required
    │
    ├─→ Does code handle user input?
    │   ├─→ YES → Apply Input Validation Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code access database?
    │   ├─→ YES → Apply SQL Injection Prevention
    │   └─→ NO → Continue
    │
    ├─→ Does code handle authentication/authorization?
    │   ├─→ YES → Apply Authentication Security Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code process sensitive data?
    │   ├─→ YES → Apply Data Protection Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code interact with external systems?
    │   ├─→ YES → Apply API Security Checklist
    │   └─→ NO → Continue
    │
    └─→ Run SAST tools (SonarCloud, CodeQL)
        └─→ Document findings and mitigation

OWASP Top 10 Security Review Checklist

A01:2021 – Broken Access Control

What to Check:

✅ Verify authorization checks exist before resource access
✅ Ensure users cannot access resources outside their permissions
✅ Check for insecure direct object references (IDOR)
✅ Validate that horizontal and vertical privilege escalation is prevented
✅ Confirm Spring Security annotations (@PreAuthorize, @Secured) are correctly applied

Code Patterns:

✅ SECURE - Proper Authorization Check:

@Service
public class DocumentService {
    @Autowired
    private SecurityContext securityContext;
    
    @PreAuthorize("hasRole('USER')")
    public Document getDocument(Long documentId) {
        Document doc = documentRepository.findById(documentId)
            .orElseThrow(() -> new ResourceNotFoundException("Document not found"));
        
        // Verify ownership before returning
        if (!doc.getOwnerId().equals(getCurrentUserId())) {
            throw new AccessDeniedException("Cannot access document owned by another user");
        }
        
        return doc;
    }
}

❌ INSECURE - Missing Authorization:

// BAD: No authorization check, anyone can access any document
@GetMapping("/documents/{id}")
public Document getDocument(@PathVariable Long id) {
    return documentRepository.findById(id).orElse(null);
}

A02:2021 – Cryptographic Failures

What to Check:

✅ Verify sensitive data is encrypted at rest and in transit
✅ Ensure strong encryption algorithms (AES-256, RSA-2048+)
✅ Check that passwords are hashed with bcrypt/argon2 (never plain text)
✅ Validate TLS 1.2+ is enforced for all connections
✅ Confirm encryption keys are managed securely (never hardcoded)

Code Patterns:

✅ SECURE - Proper Password Hashing:

@Service
public class UserService {
    @Autowired
    private PasswordEncoder passwordEncoder; // BCryptPasswordEncoder
    
    public void createUser(String username, String password) {
        String hashedPassword = passwordEncoder.encode(password);
        User user = new User(username, hashedPassword);
        userRepository.save(user);
    }
    
    public boolean verifyPassword(String rawPassword, String hashedPassword) {
        return passwordEncoder.matches(rawPassword, hashedPassword);
    }
}

❌ INSECURE - Plain Text Password:

// BAD: Storing passwords in plain text
public void createUser(String username, String password) {
    User user = new User(username, password); // NEVER DO THIS
    userRepository.save(user);
}

A03:2021 – Injection

What to Check:

✅ All SQL queries use parameterized statements or JPA criteria queries
✅ User input is validated before use in queries
✅ Command injection is prevented (no Runtime.exec with user input)
✅ LDAP, XML, and OS command injection vectors are addressed
✅ ORM queries are not constructed with string concatenation

Code Patterns:

✅ SECURE - Parameterized Query:

@Repository
public interface PoliticianRepository extends JpaRepository<Politician, Long> {
    // JPA query with named parameter - safe from SQL injection
    @Query("SELECT p FROM Politician p WHERE p.firstName = :firstName AND p.lastName = :lastName")
    List<Politician> findByName(@Param("firstName") String firstName, 
                                @Param("lastName") String lastName);
}

// Using JPA Criteria API - also safe
public List<Politician> searchPoliticians(String searchTerm) {
    CriteriaBuilder cb = entityManager.getCriteriaBuilder();
    CriteriaQuery<Politician> query = cb.createQuery(Politician.class);
    Root<Politician> politician = query.from(Politician.class);
    
    query.where(cb.like(politician.get("firstName"), "%" + searchTerm + "%"));
    return entityManager.createQuery(query).getResultList();
}

❌ INSECURE - SQL Injection Vulnerable:

// BAD: String concatenation in query
@Query(value = "SELECT * FROM politician WHERE first_name = '" + firstName + "'", 
       nativeQuery = true)
List<Politician> findByName(String firstName); // SQL INJECTION RISK!

// BAD: Direct JDBC with concatenation
public List<Politician> search(String name) {
    String sql = "SELECT * FROM politician WHERE name = '" + name + "'";
    return jdbcTemplate.query(sql, new PoliticianMapper()); // VULNERABLE!
}

A04:2021 – Insecure Design

What to Check:

✅ Security requirements defined before implementation
✅ Threat modeling completed for sensitive features
✅ Security controls are built into the architecture
✅ Rate limiting implemented for sensitive operations
✅ Business logic flaws identified and mitigated

Secure Design Principles:

// Example: Rate limiting for login attempts
@Service
public class LoginService {
    private static final int MAX_ATTEMPTS = 5;
    private static final Duration LOCKOUT_DURATION = Duration.ofMinutes(15);
    
    private final LoadingCache<String, Integer> loginAttempts = CacheBuilder.newBuilder()
        .expireAfterWrite(LOCKOUT_DURATION)
        .build(new CacheLoader<String, Integer>() {
            public Integer load(String key) {
                return 0;
            }
        });
    
    public void login(String username, String password) {
        // Check if account is locked
        int attempts = loginAttempts.get(username);
        if (attempts >= MAX_ATTEMPTS) {
            throw new AccountLockedException(
                "Account locked due to too many failed attempts. Try again in 15 minutes.");
        }
        
        // Attempt authentication
        boolean authenticated = authenticate(username, password);
        
        if (!authenticated) {
            loginAttempts.put(username, attempts + 1);
            throw new BadCredentialsException("Invalid credentials");
        }
        
        // Success - reset counter
        loginAttempts.invalidate(username);
    }
}

A05:2021 – Security Misconfiguration

What to Check:

✅ Default credentials are changed
✅ Error messages don't leak sensitive information
✅ Debug mode disabled in production
✅ Unnecessary features/services are disabled
✅ Security headers configured (CSP, HSTS, X-Frame-Options)
✅ Dependency versions are up to date

Configuration Checklist:

✅ SECURE - Production Configuration:

# application-production.yml
spring:
  profiles:
    active: production
  
  # Disable debug endpoints
  boot:
    admin:
      client:
        enabled: false
  
  # Secure session management
  session:
    timeout: 30m
    cookie:
      secure: true
      http-only: true
      same-site: strict
  
  # Hide implementation details
  mvc:
    throw-exception-if-no-handler-found: true
  resources:
    add-mappings: false

# Security headers
server:
  error:
    include-message: never
    include-stacktrace: never
    include-exception: false

❌ INSECURE - Development Settings in Production:

# BAD: Debug enabled in production
spring:
  profiles:
    active: development
  
  # Exposes sensitive endpoints
  boot:
    admin:
      client:
        enabled: true
  
  # Leak stack traces
server:
  error:
    include-stacktrace: always
    include-exception: true

A06:2021 – Vulnerable and Outdated Components

What to Check:

✅ All dependencies scanned with OWASP Dependency-Check
✅ No dependencies with known high/critical CVEs
✅ Dependencies are from trusted sources
✅ Unused dependencies are removed
✅ Security patches applied promptly

Maven Security Check:

# Run OWASP Dependency Check before adding new dependencies
mvn org.owasp:dependency-check-maven:check

# Review the report
ls -la target/dependency-check-report.html

# Fail build on high severity vulnerabilities
mvn verify -Dowasp.failOnCVSS=7

pom.xml Best Practices:

<!-- Use dependencyManagement to control versions -->
<dependencyManagement>
    <dependencies>
        <dependency>
            <groupId>org.springframework</groupId>
            <artifactId>spring-framework-bom</artifactId>
            <version>5.3.34</version>
            <type>pom</type>
            <scope>import</scope>
        </dependency>
    </dependencies>
</dependencyManagement>

<!-- Enable OWASP Dependency Check -->
<plugin>
    <groupId>org.owasp</groupId>
    <artifactId>dependency-check-maven</artifactId>
    <version>9.0.7</version>
    <configuration>
        <failBuildOnCVSS>7</failBuildOnCVSS>
        <suppressionFiles>
            <suppressionFile>dependency-check-suppressions.xml</suppressionFile>
        </suppressionFiles>
    </configuration>
</plugin>

A07:2021 – Identification and Authentication Failures

What to Check:

✅ Multi-factor authentication implemented for sensitive operations
✅ Session management is secure (secure cookies, timeout)
✅ Password requirements enforce strong passwords
✅ Credential stuffing protection (rate limiting, CAPTCHA)
✅ Session fixation attacks prevented

Code Patterns:

✅ SECURE - Spring Security Configuration:

@Configuration
@EnableWebSecurity
public class SecurityConfig {
    
    @Bean
    public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
        http
            .authorizeHttpRequests(auth -> auth
                .requestMatchers("/public/**").permitAll()
                .requestMatchers("/admin/**").hasRole("ADMIN")
                .anyRequest().authenticated()
            )
            .formLogin(form -> form
                .loginPage("/login")
                .failureHandler(authenticationFailureHandler())
                .successHandler(authenticationSuccessHandler())
            )
            .logout(logout -> logout
                .logoutSuccessUrl("/login?logout")
                .invalidateHttpSession(true)
                .deleteCookies("JSESSIONID")
            )
            .sessionManagement(session -> session
                .sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)
                .maximumSessions(1)
                .expiredUrl("/login?expired")
            )
            .csrf(csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()));
        
        return http.build();
    }
    
    @Bean
    public PasswordEncoder passwordEncoder() {
        // BCrypt with strength 12
        return new BCryptPasswordEncoder(12);
    }
}

A08:2021 – Software and Data Integrity Failures

What to Check:

✅ Deserialization of untrusted data is prevented
✅ Code signing and integrity verification in CI/CD
✅ Dependency checksums verified
✅ No auto-updates without integrity verification
✅ Git commits are signed

Secure Deserialization:

// Use safe deserialization with explicit type validation
@Bean
public ObjectMapper objectMapper() {
    ObjectMapper mapper = new ObjectMapper();
    
    // Do NOT use activateDefaultTyping for untrusted input
    // If polymorphism is required, use @JsonTypeInfo on specific DTOs
    // with a strict allowlist-based PolymorphicTypeValidator
    
    // Adjust serialization behavior if needed
    mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
    // Keep FAIL_ON_UNKNOWN_PROPERTIES enabled by default to catch unexpected/malicious fields
    
    return mapper;
}

A09:2021 – Security Logging and Monitoring Failures

What to Check:

✅ Authentication/authorization failures are logged
✅ High-value transactions are audited
✅ Logs don't contain sensitive data (passwords, tokens)
✅ Log injection attacks prevented
✅ Alerting configured for suspicious activities

Secure Logging Pattern:

@Component
public class SecurityAuditLogger {
    private static final Logger auditLog = LoggerFactory.getLogger("SECURITY_AUDIT");
    
    public void logAuthenticationSuccess(String username, String ipAddress) {
        auditLog.info("Authentication successful - User: {}, IP: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(ipAddress));
    }
    
    public void logAuthenticationFailure(String username, String ipAddress, String reason) {
        auditLog.warn("Authentication failed - User: {}, IP: {}, Reason: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(ipAddress),
            reason);
    }
    
    public void logAccessDenied(String username, String resource) {
        auditLog.warn("Access denied - User: {}, Resource: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(resource));
    }
    
    // Prevent log injection
    private String sanitizeForLog(String input) {
        if (input == null) return "null";
        return input.replaceAll("[\n\r\t]", "_");
    }
}

A10:2021 – Server-Side Request Forgery (SSRF)

What to Check:

✅ URLs from user input are validated against allowlist
✅ Network segmentation prevents access to internal resources
✅ DNS rebinding attacks prevented
✅ Disable unused URL schemas (file://, gopher://)

SSRF Prevention:

@Service
public class ExternalDataService {
    private static final Set<String> ALLOWED_HOSTS = Set.of(
        "api.riksdagen.se",
        "api.worldbank.org",
        "data.val.se"
    );
    
    public String fetchExternalData(String url) throws IOException {
        URL parsedUrl;
        try {
            parsedUrl = new URL(url);
        } catch (MalformedURLException e) {
            throw new IllegalArgumentException("Invalid URL", e);
        }
        
        // Validate protocol
        if (!parsedUrl.getProtocol().equals("https")) {
            throw new IllegalArgumentException("Only HTTPS URLs allowed");
        }
        
        // Validate host against allowlist
        if (!ALLOWED_HOSTS.contains(parsedUrl.getHost())) {
            throw new IllegalArgumentException("Host not in allowlist: " + parsedUrl.getHost());
        }
        
        // Fetch data with timeout
        HttpURLConnection conn = (HttpURLConnection) parsedUrl.openConnection();
        conn.setConnectTimeout(5000);
        conn.setReadTimeout(5000);
        
        return IOUtils.toString(conn.getInputStream(), StandardCharsets.UTF_8);
    }
}

SAST/DAST Integration

CodeQL Configuration

Ensure .github/workflows/codeql.yml includes:

name: "CodeQL Security Analysis"
on:
  push:
    branches: [ main, develop ]
  pull_request:
    branches: [ main ]
  schedule:
    - cron: '0 0 * * 0' # Weekly scan

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest
    permissions:
      actions: read
      contents: read
      security-events: write
    
    steps:
    - name: Checkout repository
      uses: actions/checkout@v4
    
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v3
      with:
        languages: java
        queries: security-and-quality
    
    - name: Build
      run: mvn clean compile -DskipTests
    
    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v3

SonarCloud Quality Gates

Required quality gate thresholds:

✅ Security Rating: A (0 vulnerabilities)
✅ Security Hotspots: All reviewed
✅ Coverage: > 80%
✅ Duplications: < 3%
✅ Maintainability Rating: A

ISMS Compliance Mapping

ISO 27001:2022 Controls

A.8.1 - User Endpoint Devices: Secure code prevents endpoint exploitation
A.8.2 - Privileged Access Rights: Authorization checks enforce least privilege
A.8.3 - Information Access Restriction: Access control mechanisms properly implemented
A.8.8 - Management of Technical Vulnerabilities: Vulnerability scanning and remediation
A.8.28 - Secure Coding: Adherence to secure coding standards

NIST Cybersecurity Framework

PR.DS-6: Integrity checking mechanisms in place
PR.IP-1: Baseline security configurations
DE.CM-4: Malicious code detected
RS.AN-5: Processes for vulnerability response

CIS Controls v8

Control 16.11: Remediate detected vulnerabilities
Control 4.1: Establish and maintain secure configuration
Control 16.1: Application software security
Control 16.14: Establish process for accepting application risk

Hack23 ISMS Policy References

Review these policies before code review:

Secure Development Policy: https://github.com/Hack23/ISMS-PUBLIC/blob/main/policies/secure-development-policy.md
Vulnerability Management Policy: https://github.com/Hack23/ISMS-PUBLIC/blob/main/policies/vulnerability-management-policy.md
Access Control Policy: https://github.com/Hack23/ISMS-PUBLIC/blob/main/policies/access-control-policy.md

Review Workflow

Pre-Review Setup

# Checkout PR branch
gh pr checkout <PR-NUMBER>

# Run security scans
mvn clean verify
mvn org.owasp:dependency-check-maven:check

# Review CodeQL alerts
gh api /repos/Hack23/cia/code-scanning/alerts --jq '.[] | select(.state=="open")'

Manual Code Review
- Apply OWASP Top 10 checklist to changed files
- Review authentication/authorization changes
- Verify input validation on all user inputs
- Check for hardcoded secrets or credentials
- Validate error handling doesn't leak sensitive info

Document Findings

## Security Review Findings

### Critical Issues
- [ ] SQL injection in `PoliticianController.search()` - line 45

### High Priority
- [ ] Missing authorization check in `DocumentService.getDocument()`

### Medium Priority
- [ ] Weak password validation in `UserRegistrationForm`

### Low Priority / Informational
- [ ] Consider adding rate limiting to login endpoint

Request Changes or Approve
- If critical/high issues found: Request changes with detailed explanation
- If only medium/low: Approve with recommendations
- Always provide actionable feedback with code examples

Security Review Exit Criteria

Before approving a PR, verify:

✅ No new CodeQL alerts introduced
✅ SonarCloud quality gate passed
✅ OWASP Dependency Check shows no new high/critical CVEs
✅ All authentication/authorization logic reviewed
✅ Input validation present on all user inputs
✅ No secrets or credentials in code
✅ Security test cases added for sensitive features
✅ Documentation updated if security architecture changed

Hack23 ISMS Policy References

Secure Development Framework:

Secure Development Policy - Comprehensive SDLC security requirements
Information Security Policy - Master governance and security principles
Cryptography Policy - Approved algorithms and key management
Vulnerability Management - SLAs and remediation procedures
Threat Modeling - STRIDE framework and attack trees
Change Management - Secure change control procedures
Incident Response Plan - Security incident handling

All Hack23 ISMS Policies: https://github.com/Hack23/ISMS-PUBLIC

CIA Platform Architecture References

Security Architecture: CIA SECURITY_ARCHITECTURE.md - Current security controls
Future Security: CIA FUTURE_SECURITY_ARCHITECTURE.md - Planned enhancements
Threat Model: CIA THREAT_MODEL.md - STRIDE analysis and attack trees
Architecture: CIA ARCHITECTURE.md - System design
Security Policy: CIA SECURITY.md - Vulnerability disclosure

References

Official Standards

OWASP Top 10: https://owasp.org/www-project-top-ten/
OWASP ASVS: https://owasp.org/www-project-application-security-verification-standard/
CWE Top 25: https://cwe.mitre.org/top25/
SANS Top 25: https://www.sans.org/top25-software-errors/
ISO 27001:2022: https://www.iso.org/standard/27001
NIST SP 800-53: https://csrc.nist.gov/publications/detail/sp/800-53/rev-5/final

Tools & Frameworks

CodeQL: https://codeql.github.com/
SonarCloud: https://sonarcloud.io/
OWASP Dependency Check: https://owasp.org/www-project-dependency-check/
SpotBugs with FindSecBugs: https://find-sec-bugs.github.io/
Spring Security: https://docs.spring.io/spring-security/reference/

Success Metrics

Track these KPIs to measure secure code review effectiveness:

Zero security vulnerabilities in production
Mean time to remediate (MTTR) for vulnerabilities < 30 days
100% of PRs pass security review before merge
Decrease in vulnerability count over time
Security training completion rate for all contributors

Install

npx skillscat add hack23/cia/secure-code-review

Install via the SkillsCat registry.

Repository Stars

The star count shown is for the parent repository (Hack23/cia), not this specific skill.

.github/skills/secure-code-review

secure-code-review

Install

Secure Code Review Skill

Purpose

When to Use This Skill

Decision Tree

OWASP Top 10 Security Review Checklist

A01:2021 – Broken Access Control

A02:2021 – Cryptographic Failures

A03:2021 – Injection

A04:2021 – Insecure Design

A05:2021 – Security Misconfiguration

A06:2021 – Vulnerable and Outdated Components

A07:2021 – Identification and Authentication Failures

A08:2021 – Software and Data Integrity Failures

A09:2021 – Security Logging and Monitoring Failures

A10:2021 – Server-Side Request Forgery (SSRF)

SAST/DAST Integration

CodeQL Configuration

SonarCloud Quality Gates

ISMS Compliance Mapping

ISO 27001:2022 Controls

NIST Cybersecurity Framework

CIS Controls v8

Hack23 ISMS Policy References

Review Workflow

Security Review Exit Criteria

Hack23 ISMS Policy References

CIA Platform Architecture References

References

Official Standards

Tools & Frameworks

Success Metrics

Categories

Install

Repository Stars

Recommended Skills