SQL injection vulnerabilities arise in applications where elements of a SQL query originate from an untrusted source. Without precautions, the untrusted data may maliciously alter the query, resulting in information leaks or data modification. The primary means of preventing SQL injection are sanitization and validation, which are typically implemented as parameterized queries and stored procedures.
Suppose a system authenticates users by issuing the following query to a SQL database. If the query returns any results, authentication succeeds; otherwise, authentication fails.
SELECT * FROM db_user WHERE username='<USERNAME>' AND password='<PASSWORD>'
Suppose an attacker can substitute arbitrary strings for <USERNAME>
and <PASSWORD>
. In that case, the authentication mechanism can be bypassed by supplying the following <USERNAME>
with an arbitrary password:
validuser' OR '1'='1
The authentication routine dynamically constructs the following query:
SELECT * FROM db_user WHERE username='validuser' OR '1'='1' AND password='<PASSWORD>'
If validuser
is a valid user name, this SELECT
statement yields the validuser
record in the table. The password is never checked because username='validuser'
is true; consequently, the items after the OR
are not tested. As long as the components after the OR
generate a syntactically correct SQL expression, the attacker is granted the access of validuser
.
Similarly, an attacker could supply the following string for <PASSWORD>
with an arbitrary username:
' OR '1'='1
producing the following query:
SELECT * FROM db_user WHERE username='<USERNAME>' AND password='' OR '1'='1'
'1'='1'
always evaluates to true, causing the query to yield every row in the database. In this scenario, the attacker would be authenticated without needing a valid username or password.
Noncompliant Code Example
This noncompliant code example shows JDBC code to authenticate a user to a system. The password is passed as a char
array, the database connection is created, and then the passwords are hashed.
Unfortunately, this code example permits a SQL injection attack by incorporating the unsanitized input argument username
into the SQL command, allowing an attacker to inject validuser' OR '1'='1
. The password
argument cannot be used to attack this program because it is passed to the hashPassword()
function, which also sanitizes the input.
import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; class Login { public Connection getConnection() throws SQLException { DriverManager.registerDriver(new com.microsoft.sqlserver.jdbc.SQLServerDriver()); String dbConnection = PropertyManager.getProperty("db.connection"); // Can hold some value like // "jdbc:microsoft:sqlserver://<HOST>:1433,<UID>,<PWD>" return DriverManager.getConnection(dbConnection); } String hashPassword(char[] password) { // Create hash of password } public void doPrivilegedAction(String username, char[] password) throws SQLException { Connection connection = getConnection(); if (connection == null) { // Handle error } try { String pwd = hashPassword(password); String sqlString = "SELECT * FROM db_user WHERE username = '" + username + "' AND password = '" + pwd + "'"; Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(sqlString); if (!rs.next()) { throw new SecurityException( "User name or password incorrect" ); } // Authenticated; proceed } finally { try { connection.close(); } catch (SQLException x) { // Forward to handler } } } }
Noncompliant Code Example (PreparedStatement
)
The JDBC library provides an API for building SQL commands that sanitize untrusted data. The java.sql.PreparedStatement
class properly escapes input strings, preventing SQL injection when used correctly. This code example modifies the doPrivilegedAction()
method to use a PreparedStatement
instead of java.sql.Statement
. However, the prepared statement still permits a SQL injection attack by incorporating the unsanitized input argument username
into the prepared statement.
import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; class Login { public Connection getConnection() throws SQLException { DriverManager.registerDriver(new com.microsoft.sqlserver.jdbc.SQLServerDriver()); String dbConnection = PropertyManager.getProperty("db.connection"); // Can hold some value like // "jdbc:microsoft:sqlserver://<HOST>:1433,<UID>,<PWD>" return DriverManager.getConnection(dbConnection); } String hashPassword(char[] password) { // Create hash of password } public void doPrivilegedAction( String username, char[] password ) throws SQLException { Connection connection = getConnection(); if (connection == null) { // Handle error } try { String pwd = hashPassword(password); String sqlString = "select * from db_user where username=" + username + " and password =" + pwd; PreparedStatement stmt = connection.prepareStatement(sqlString); ResultSet rs = stmt.executeQuery(); if (!rs.next()) { throw new SecurityException("User name or password incorrect"); } // Authenticated; proceed } finally { try { connection.close(); } catch (SQLException x) { // Forward to handler } } } }
Compliant Solution (PreparedStatement
)
This compliant solution uses a parametric query with a ?
character as a placeholder for the argument. This code also validates the length of the username
argument, preventing an attacker from submitting an arbitrarily long user name.
public void doPrivilegedAction( String username, char[] password ) throws SQLException { Connection connection = getConnection(); if (connection == null) { // Handle error } try { String pwd = hashPassword(password); // Validate username length if (username.length() > 8) { // Handle error } String sqlString = "select * from db_user where username=? and password=?"; PreparedStatement stmt = connection.prepareStatement(sqlString); stmt.setString(1, username); stmt.setString(2, pwd); ResultSet rs = stmt.executeQuery(); if (!rs.next()) { throw new SecurityException("User name or password incorrect"); } // Authenticated; proceed } finally { try { connection.close(); } catch (SQLException x) { // Forward to handler } } }
Use the set*()
methods of the PreparedStatement
class to enforce strong type checking. This technique mitigates the SQL injection vulnerability because the input is properly escaped by automatic entrapment within double quotes. Note that prepared statements must be used even with queries that insert data into the database.
Risk Assessment
Failure to sanitize user input before processing or storing it can result in injection attacks.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
IDS00-J | High | Likely | Medium | P18 | L1 |
Automated Detection
Tool | Version | Checker | Description |
---|---|---|---|
The Checker Framework | 2.1.3 | Tainting Checker | Trust and security errors (see Chapter 8) |
CodeSonar | 8.1p0 | JAVA.IO.INJ.SQL | SQL Injection (Java) |
Coverity | 7.5 | SQLI | Implemented |
Findbugs | 1.0 | SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE | Implemented |
Fortify | 1.0 | HTTP_Response_Splitting | Implemented |
Klocwork | 2024.3 | SV.DATA.DB | Implemented |
Parasoft Jtest | 2024.1 | CERT.IDS00.TDSQL | Protect against SQL injection |
SonarQube | 9.9 | ||
SpotBugs | 4.6.0 | SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE | Implemented |
Related Vulnerabilities
CVE-2008-2370 describes a vulnerability in Apache Tomcat 4.1.0 through 4.1.37, 5.5.0 through 5.5.26, and 6.0.0 through 6.0.16. When a RequestDispatcher
is used, Tomcat performs path normalization before removing the query string from the URI, which allows remote attackers to conduct directory traversal attacks and read arbitrary files via a ..
(dot dot) in a request parameter.
Related Guidelines
SEI CERT Perl Coding Standard | IDS33-PL. Sanitize untrusted data passed across a trust boundary |
Injection [RST] | |
CWE-116, Improper Encoding or Escaping of Output |
Android Implementation Details
This rule uses Microsoft SQL Server as an example to show a database connection. However, on Android, DatabaseHelper
from SQLite is used for a database connection. Because Android apps may receive untrusted data via network connections, the rule is applicable.
Bibliography
A Guide to Building Secure Web Applications and Web Services | |
[Seacord 2015] | IDS00-J. Prevent SQL Injection LiveLesson |
[W3C 2008] | Section 4.4.3, "Included If Validating" |
23 Comments
Dhruv Mohindra
I recommend that schema validation as described in the old dedicated xml injection guideline should be mentioned/incorporated here. You cannot whitelist an XML document that you receive on a wire (which is the case usually).
David Svoboda
Sure you can. That is, you could implement the CS on an XML stream you get over the net. Or you could at least use whitelisting to validate the quantity.
Fixed
Also fixed
Dhruv Mohindra
You can try to use regexs for validation of XML on wire but it is quite common to have XMLs that are very large. Regex validation for XML docs is just far fetched (I don't mean that you can't do it). The first question anyone dealing with XML asks is - is there any way to validate the XML. If yes, the dirty work from the mentioned CS can be reduced.
Dhruv Mohindra
Just wanted to add that schema validation can be expensive performance wise so it can't be suggested as a CS (some vendors can't afford to provide it), but perhaps as an exception to the rule? Even secure XML data exchange over HTTPS?
David Svoboda
Sure it can be suggested; I've just done it It may not be a fast option, but it is an option.
I Gede Panca Sutresna
Hi there..
I found that there's also CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') and CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') that related to this guideline. So maybe we can add two of this CWE's item in the Related Guidelines section.
Regards,
Paul Evans
This rule does not seem to cover JPA context.
In our architecture, we use JPA and not JDBC.
We have implemented a PMD rule for IDS00-J, which checks the query method.
Does this seem to be a correct interpretation of IDS00-J? Can you offer more advice in a JPA context please?
David Svoboda
Dhruv Mohindra says:
James Ahlborn
I'm disappointed by the "solutions" proposed for XML injection. In general, generating XML via String concatenation is a bad idea. The solution to XML injection is the same as the solution to SQL injection, use the tools which already handle this for you. Like PreparedStatement for SQL, if you build your XML using DOM (or StAX, SAX, etc.) then any text values that you insert should be correctly escaped by the framework.
Schema validation is a great way to check arbitrary XML received from an untrusted source, but if you are building the XML yourself, you don't need to validate after generation if you use one of the purpose built XML builder tools (DOM,StAX,etc.).
Dean Sutherland
Using one of the frameworks that provide checked construction of XML is certainly a good idea – the escaping is clearly desirable. I agree that we should at least mention the type-safe framework approach.
That said, we were attempting to demonstrate a variety of solutions across a variety of different classes of sanitization. Schema validation has the charm that it is different from the PreparedStatement approach. As you point out, using DOM (or StAX or SAX) to build the XML is essentially similar to the SQL solution. We used the example you complain about exactly because it demonstrates a completely different approach.
Of course, we cannot cover the complete space of acceptable solutions. The examples and compliant solutions are consequently illustrative rather than normative. The normative portion of this rule is:
"Such data must be sanitized both because the subsystem may be unprepared to handle the malformed input and because unsanitized input may include an injection attack.
In particular, programs must sanitize all string data that is passed to command interpreters or parsers so that the resulting string is innocuous in the context in which it is parsed or interpreted."
All else is illustration and discussion.
James Ahlborn
While i can see your point, the beginning of the book describes this book as a set of secure coding practices. in my reading of the book, i am approaching the guide as a collection of best practices for java coding. i would imagine most others reading this book will have the same perspective. the average developer who determines that they need to make sure their xml handling is secure is going to come upon this website (or open up their copy of the book) and arrive at this chapter. the will skip the the section on "xml injection" and conclude that the way to write secure xml is using string concatenation and regexes. this is a terrible conclusion. string concatenation should be a last resort and, if absolutely necessary, then sure, the techniques currently listed here (for xml handling) could be useful. i doubt many developers will start reading from the beginning of the section, read through the section on preventing SQL injection, and conclude that they should use a DOMBuilder for secure xml creation. especially if they do not already happen to be familiar with the xml tools provided by the jdk.
as for Schema validation, it is certainly a different approach but, i would argue, largely mis-applied here. you shouldn't often have to validate xml you generate yourself, if you do, you are most likely doing it wrong (the xml generation). xml validation is for handling untrusted xml documents. if i do the bulk of the generation myself (using the correct tools), i should have no cause to mistrust it (otherwise, nothing is safe).
i agree that you cannot possibly cover the complete space of acceptable solutions (or even of all possible problems), but i would imagine that for the illustrations you do include, you should at least include a best practice solution. additionally, sql injection and xml injection are problem spaces that a large number of developers probably face in day to day development, so not including best practices in those specific areas does a disservice to this invaluable guide.
Yitzhak Mandelbaum
With all due respect, I have to disagree. "Nothing is safe" is generally a good attitude towards code, yours or others. If that's the message we're conveying in our example, then I daresay we're doing our job.
Unless you've verified your code it has bugs in it.
James Ahlborn
Well, the schema discussion is kind of secondary to my main point. that said, if nothing is safe, why would i trust the schema validation code? if i can't trust anything i can't really write any code at all. My code has bugs in it. The schema validation code has bugs in it. all non-trivial code has bugs in it. i didn't understand that elucidating that point was the point of the book.
Yitzhak Mandelbaum
I don't mean to imply that its the point of the book, only that its an important perspective when attempting to write code securely. However, i've clearly ventured off-topic here. Best to save discussions of this sort for elsewhere.
Yitzhak Mandelbaum
While I see your point, it would seem to be an oversight in our introduction, rather than a problem with this example. Just to echo/amplify Dean's earlier point – in practice, the code examples are necessary to crystallize the meaning of the rule. Sometimes that requires venturing outside of best practices just to clarify a point.
James Ahlborn
That's good to know. I will adjust my perspective as i read the rest of the book. i would recommend emphasizing this point strongly on this wiki, as others might mistake the "compliant code" examples as recommendations of "good code".
Yitzhak Mandelbaum
Agreed. That said, please do continue pointing out compliant code which you find suboptimal.
James Ahlborn
sure thing. i've already posted about an incorrect solution here.
Robert Seacord
The organization of the various sanitization rules continues to confound me. This one emphasizes "command interpreters" in the description but contains no such examples. Instead, these can be found in IDS07-J. Do not pass untrusted, unsanitized data to the Runtime.exec() method.
Would it be better to split this up into three separate rules for SQL injection, XML Injection, and XML External Entity Attacks?
David Svoboda
This rule was originally split up as you are proposing. We had an 'sql injection' rule and an 'xml injection' rule. They got demoted to recommendations to save space in the book.
Robert Seacord
We seem to have these guidelines:
This is also an injection rule (log injection):
I can also find a voided:
I'm not sure I understand the "space" argument. I guess perhaps it would be slightly longer with three Risk Assessments and so forth.
I guess I would vote for breaking these back up. It is a pretty random collection as it stands. What do you think?
The next question would be why are some of these guidelines and some rules?
Final question would be why doesn't IDS07-J fit the pattern, e.g., "Prevent command injection"
David Svoboda
IIRC the space argument is that we had a space constraint on the first Java book, we didn't want it to exceed 750 pages. At the time we had 5-6 rules that said "prevent XXX injection". IDS00-J was a generalization of all these rules, and addressed all these problems from the theoretical basis of data sanitization and external interpreters (eg SQL database, HTML web browser, etc). IDS07-J is still one of these rules; it addresses command injection (even if it does not contain that phrase.)
So to save space, we eliminated the SQL and XML rules, folding their examples into IDS00-J. We also demoted the other rules to guidelines, as you cite. Not because they are any less valid that the IDS rules we preserved, but simply because they are less prevalent.
As for what to do in the future, my suggestion is that we keep IDS00-J as a generalized version of all the 'prevent XXX injection' rules. We could resurrect the SQL and XML injection rules, leaving IDS00-J without code examples. IMO all the guidelines should be promoted to rules (as long as we don't care about the size of the rules).
Robert Seacord
I definitely think we should forget about size for now and focus on getting things right. There are almost enough injection rules that they could have their own section/chapter (INJ). The generalized rules could go into the introduction were we already have a section on injection attacks. Sound like a plan?