-
Notifications
You must be signed in to change notification settings - Fork 0
Create Java application with intentional vulnerabilities and CodeQL autobuild workflow #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create Java application with intentional vulnerabilities and CodeQL autobuild workflow #2
Conversation
…workflow Co-authored-by: mickeygousset <20031479+mickeygousset@users.noreply.github.com>
…rkflow Co-authored-by: mickeygousset <20031479+mickeygousset@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
"' AND password = '" + password + "'"; | ||
|
||
System.out.println("Executing query: " + query); | ||
ResultSet rs = stmt.executeQuery(query); |
Check failure
Code scanning / CodeQL
Query built by concatenation with a possibly-untrusted string High
this expression
Query built by concatenation with
this expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix the problem, all SQL queries using user input must be refactored to use parameterized/prepared statements rather than direct string concatenation. Specifically for the error on line 32 (and similar patterns elsewhere), the query should be changed to include ?
placeholders for inputs, and then the input values set using the appropriate setString
methods before execution. This means changing the authenticateUser
method to use a PreparedStatement
, constructing the query as "SELECT * FROM users WHERE username = ? AND password = ?"
, then setting the values for both placeholders using setString(1, username)
and setString(2, password)
respectively. Similar changes should be made to the other methods (updateUserProfile
, deleteUser
) to replace concatenated statements with parameterized queries, using PreparedStatement
and passing the variables safely to the SQL engine.
Changes should occur only in code snippets shown (within src/main/java/com/example/database/UserDatabase.java), and only in the affected methods. Additional required imports (for PreparedStatement
) should also be added. Functionality is preserved: the logic now runs with safe, parameterized queries.
-
Copy modified line R7 -
Copy modified lines R26-R29 -
Copy modified lines R31-R32 -
Copy modified line R37 -
Copy modified lines R49-R53 -
Copy modified line R55 -
Copy modified line R57 -
Copy modified lines R69-R72 -
Copy modified line R74
@@ -4,6 +4,7 @@ | ||
import java.sql.DriverManager; | ||
import java.sql.ResultSet; | ||
import java.sql.Statement; | ||
import java.sql.PreparedStatement; | ||
|
||
/** | ||
* Database class with intentional SQL injection vulnerabilities | ||
@@ -22,19 +23,18 @@ | ||
public boolean authenticateUser(String username, String password) { | ||
try { | ||
Connection conn = DriverManager.getConnection(DB_URL, DB_USER, DB_PASSWORD); | ||
Statement stmt = conn.createStatement(); | ||
String query = "SELECT * FROM users WHERE username = ? AND password = ?"; | ||
PreparedStatement pstmt = conn.prepareStatement(query); | ||
pstmt.setString(1, username); | ||
pstmt.setString(2, password); | ||
|
||
// VULNERABILITY: Direct string concatenation leads to SQL injection | ||
String query = "SELECT * FROM users WHERE username = '" + username + | ||
"' AND password = '" + password + "'"; | ||
System.out.println("Executing query (safe, parameterized): " + query); | ||
ResultSet rs = pstmt.executeQuery(); | ||
|
||
System.out.println("Executing query: " + query); | ||
ResultSet rs = stmt.executeQuery(query); | ||
|
||
boolean authenticated = rs.next(); | ||
|
||
rs.close(); | ||
stmt.close(); | ||
pstmt.close(); | ||
conn.close(); | ||
|
||
return authenticated; | ||
@@ -51,16 +46,15 @@ | ||
public void updateUserProfile(String userId, String email, String fullName) { | ||
try { | ||
Connection conn = DriverManager.getConnection(DB_URL, DB_USER, DB_PASSWORD); | ||
Statement stmt = conn.createStatement(); | ||
String updateQuery = "UPDATE users SET email = ?, full_name = ? WHERE user_id = ?"; | ||
PreparedStatement pstmt = conn.prepareStatement(updateQuery); | ||
pstmt.setString(1, email); | ||
pstmt.setString(2, fullName); | ||
pstmt.setString(3, userId); | ||
|
||
// VULNERABILITY: String concatenation in UPDATE statement | ||
String updateQuery = "UPDATE users SET email = '" + email + | ||
"', full_name = '" + fullName + | ||
"' WHERE user_id = " + userId; | ||
pstmt.executeUpdate(); | ||
|
||
stmt.executeUpdate(updateQuery); | ||
|
||
stmt.close(); | ||
pstmt.close(); | ||
conn.close(); | ||
|
||
} catch (Exception e) { | ||
@@ -74,13 +66,12 @@ | ||
public void deleteUser(String userIdParam) { | ||
try { | ||
Connection conn = DriverManager.getConnection(DB_URL, DB_USER, DB_PASSWORD); | ||
Statement stmt = conn.createStatement(); | ||
String sql = "DELETE FROM users WHERE id = ?"; | ||
PreparedStatement pstmt = conn.prepareStatement(sql); | ||
pstmt.setString(1, userIdParam); | ||
pstmt.executeUpdate(); | ||
|
||
// VULNERABILITY: Direct concatenation in DELETE statement | ||
String sql = "DELETE FROM users WHERE id = " + userIdParam; | ||
stmt.executeUpdate(sql); | ||
|
||
stmt.close(); | ||
pstmt.close(); | ||
conn.close(); | ||
|
||
} catch (Exception e) { |
"', full_name = '" + fullName + | ||
"' WHERE user_id = " + userId; | ||
|
||
stmt.executeUpdate(updateQuery); |
Check failure
Code scanning / CodeQL
Query built by concatenation with a possibly-untrusted string High
this expression
Query built by concatenation with
this expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
General fix:
Avoid direct concatenation of potentially untrusted input into SQL queries. Instead, use prepared statements, with parameter placeholders (?
) for each value derived from user input, and provide those values with the appropriate setter methods (e.g., setString
, setInt
).
Detailed description:
Replace the creation of the Statement
with a PreparedStatement
. Change the construction of updateQuery
from using concatenation, to using a parameterized SQL statement with question mark placeholders for email
, fullName
, and userId
. Replace the call to stmt.executeUpdate(updateQuery)
with the use of the PreparedStatement
, after setting its parameters in the correct order with .setString()
or the appropriate setter. Remember to close the PreparedStatement
instead of the old Statement
.
What needs to be changed:
- Replace creation of
Statement stmt = conn.createStatement()
with aPreparedStatement
with parameters. - Rewrite the SQL to use
?
placeholders. - Set each parameter using the relevant setter.
- Replace uses of
stmt
withpreparedStatement
as appropriate. - Close the
PreparedStatement
. - Add import for
java.sql.PreparedStatement
at the top if not present.
-
Copy modified line R7 -
Copy modified lines R55-R59 -
Copy modified line R61 -
Copy modified line R63
@@ -4,6 +4,7 @@ | ||
import java.sql.DriverManager; | ||
import java.sql.ResultSet; | ||
import java.sql.Statement; | ||
import java.sql.PreparedStatement; | ||
|
||
/** | ||
* Database class with intentional SQL injection vulnerabilities | ||
@@ -51,16 +52,15 @@ | ||
public void updateUserProfile(String userId, String email, String fullName) { | ||
try { | ||
Connection conn = DriverManager.getConnection(DB_URL, DB_USER, DB_PASSWORD); | ||
Statement stmt = conn.createStatement(); | ||
String updateQuery = "UPDATE users SET email = ?, full_name = ? WHERE user_id = ?"; | ||
PreparedStatement pstmt = conn.prepareStatement(updateQuery); | ||
pstmt.setString(1, email); | ||
pstmt.setString(2, fullName); | ||
pstmt.setString(3, userId); | ||
|
||
// VULNERABILITY: String concatenation in UPDATE statement | ||
String updateQuery = "UPDATE users SET email = '" + email + | ||
"', full_name = '" + fullName + | ||
"' WHERE user_id = " + userId; | ||
pstmt.executeUpdate(); | ||
|
||
stmt.executeUpdate(updateQuery); | ||
|
||
stmt.close(); | ||
pstmt.close(); | ||
conn.close(); | ||
|
||
} catch (Exception e) { |
public String hashPassword(String password) { | ||
try { | ||
// VULNERABILITY: Using MD5 for password hashing (weak algorithm) | ||
MessageDigest md = MessageDigest.getInstance("MD5"); |
Check failure
Code scanning / CodeQL
Use of a potentially broken or risky cryptographic algorithm High
MD5
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix the detected problem, we should replace the use of MD5 in the hashPassword
method with a strong, modern password-hashing approach. The best practice in 2024 is to use a dedicated password hashing algorithm such as PBKDF2 with HMAC SHA-256, bcrypt, or Argon2. Since we are limited to the provided code snippet and can only introduce standard library imports, we should use PBKDF2 from the Java standard library (javax.crypto.SecretKeyFactory
, javax.crypto.spec.PBEKeySpec
) to hash passwords.
Thus, in CryptoUtils.java
, update the hashPassword
method to use PBKDF2WithHmacSHA256. Generate a random salt per password hash (so the hash is unique for each input) – but as we are limited in code scope and do not have storage/retrieval, for demonstration purposes we’ll use a hardcoded salt. (In real usage, you should always use a random salt per password.)
You need to:
- Add the required imports.
- Update the
hashPassword
implementation to use PBKDF2WithHmacSHA256, with a hardcoded salt and reasonable iteration count/key length. - The returned hash should be hex-encoded as before.
-
Copy modified lines R5-R8 -
Copy modified lines R53-R59 -
Copy modified line R74
@@ -2,6 +2,10 @@ | ||
|
||
import java.util.Random; | ||
import java.security.MessageDigest; | ||
import javax.crypto.SecretKeyFactory; | ||
import javax.crypto.spec.PBEKeySpec; | ||
import java.security.spec.KeySpec; | ||
import java.util.Base64; | ||
|
||
/** | ||
* Security utilities with intentional cryptographic vulnerabilities | ||
@@ -46,9 +50,13 @@ | ||
*/ | ||
public String hashPassword(String password) { | ||
try { | ||
// VULNERABILITY: Using MD5 for password hashing (weak algorithm) | ||
MessageDigest md = MessageDigest.getInstance("MD5"); | ||
byte[] hash = md.digest(password.getBytes()); | ||
// FIXED: Using PBKDF2WithHmacSHA256 for password hashing (modern algorithm) | ||
byte[] salt = "fixedSaltValue1234".getBytes(); // Use random salt in real code! | ||
int iterations = 65536; | ||
int keyLength = 256; | ||
KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, iterations, keyLength); | ||
SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); | ||
byte[] hash = factory.generateSecret(spec).getEncoded(); | ||
|
||
StringBuilder hexString = new StringBuilder(); | ||
for (byte b : hash) { | ||
@@ -63,7 +71,7 @@ | ||
|
||
} catch (Exception e) { | ||
System.err.println("Hashing failed: " + e.getMessage()); | ||
return password; // VULNERABILITY: Fallback to plaintext | ||
return password; // Still fallback: plaintext | ||
} | ||
} | ||
|
public String executeCommand(String userCommand) { | ||
try { | ||
// VULNERABILITY: Direct execution of user input | ||
Process process = Runtime.getRuntime().exec("sh -c " + userCommand); |
Check failure
Code scanning / CodeQL
Building a command line with string concatenation Critical
public String executeCommand(String userCommand) { | ||
try { | ||
// VULNERABILITY: Direct execution of user input | ||
Process process = Runtime.getRuntime().exec("sh -c " + userCommand); |
Check warning
Code scanning / CodeQL
Executing a command with a relative path Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
The best way to fix the issue is to specify the absolute path to the shell interpreter ("sh") when executing a command via Runtime.getRuntime().exec
. This means replacing "sh -c " + userCommand
with "/bin/sh -c " + userCommand
, to ensure that it explicitly runs the trusted system shell.
If sh
could be located in a different directory on some installations, consider allowing the shell path to be configurable, but by default it should use a hardcoded trusted absolute path. In this particular code, since only the relative path "sh"
is used, and the background recommends absolute paths, we should change line 81 to use "/bin/sh"
.
No additional imports or methods are needed for this particular change.
Edit required:
- src/main/java/com/example/web/FileController.java line 81:
- Change
"sh -c " + userCommand
to"/bin/sh -c " + userCommand"
- Change
-
Copy modified line R81
@@ -78,7 +78,7 @@ | ||
public String executeCommand(String userCommand) { | ||
try { | ||
// VULNERABILITY: Direct execution of user input | ||
Process process = Runtime.getRuntime().exec("sh -c " + userCommand); | ||
Process process = Runtime.getRuntime().exec("/bin/sh -c " + userCommand); | ||
|
||
StringBuilder output = new StringBuilder(); | ||
try (BufferedReader reader = new BufferedReader( |
ProcessBuilder pb = new ProcessBuilder("/bin/sh", "-c", cmd); | ||
Process process = pb.start(); | ||
|
||
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream())); |
Check warning
Code scanning / CodeQL
Potential input resource leak Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix this problem, the BufferedReader reader = ...
block and subsequent code should be wrapped in a try-with-resources statement so that the reader (and the stream it wraps) is always closed, even if an error occurs. This matches the best practice already used elsewhere in this file (e.g., in methods on lines 34—41 and 84—91).
Specifically, replace lines 110–115 such that:
BufferedReader reader = ...
becomes a try-with-resources statement, liketry (BufferedReader reader = ...) { ... }
.- The closing curly brace comes after the read loop, so that the resource is properly closed after use.
No extra imports or extra methods are required; all existing imports suffice.
-
Copy modified lines R111-R115
@@ -107,11 +107,12 @@ | ||
ProcessBuilder pb = new ProcessBuilder("/bin/sh", "-c", cmd); | ||
Process process = pb.start(); | ||
|
||
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream())); | ||
StringBuilder result = new StringBuilder(); | ||
String line; | ||
while ((line = reader.readLine()) != null) { | ||
result.append(line).append("\n"); | ||
try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { | ||
String line; | ||
while ((line = reader.readLine()) != null) { | ||
result.append(line).append("\n"); | ||
} | ||
} | ||
|
||
return result.toString(); |
Added commons-lang3 dependency version 3.12.0.
What do I need to do to get the dependencies to show up in the dependency graph |
For Maven dependencies to show up in GitHub's dependency graph, the following should be in place:
The dependency graph should populate automatically since your
Your current |
This PR implements a comprehensive Java application designed to demonstrate CodeQL security scanning capabilities, featuring multiple intentional vulnerabilities across different security categories.
Application Overview
The application is built with Maven and includes a multi-package structure demonstrating various security vulnerability patterns:
com.example.app.VulnerableApplication
) - Entry point that exercises all vulnerability typescom.example.database.UserDatabase
) - SQL injection vulnerabilities through string concatenationcom.example.security.CryptoUtils
) - Weak cryptographic implementations including MD5 and insecure randomnesscom.example.web.FileController
) - Path traversal and command injection vulnerabilitiescom.example.ldap.LdapAuth
) - LDAP injection through unescaped filter constructionVulnerability Categories Implemented
The application includes the following intentional security vulnerabilities designed to trigger CodeQL alerts:
SQL Injection
Command Injection
Path Traversal
LDAP Injection
Weak Cryptography & Security
java.util.Random
for security-sensitive token generationCodeQL Workflow Configuration
Added
.github/workflows/codeql-analysis.yml
with the following features:+security-and-quality
queries for maximum coverageThe workflow is configured to use Java 11 with the Temurin distribution and includes proper permissions for security event publishing.
Project Structure
Testing & Validation
The application successfully:
mvn clean compile
mvn test
.gitignore
for Maven projectsSecurity Notice
The implementation serves as a comprehensive test case for CodeQL's ability to detect various categories of security vulnerabilities in Java applications using the autobuild functionality.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/devopselvis/devopselvis%2Fcoding-agent-example-java-codeql-autobuild/languages
/home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js
(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.