0

We have a Java web application that was vulnerable to blind SQL injection attacks. Our developers fixed the code by using the replaceAll() function to convert single quotes to two single quotes. I am trying to understand whether the following lines of code would still be vulnerable to SQL injection attacks? 

String userInput1 = in.getuserInput1();
check = (String)form.get("userInput1");
String userInput2 = (String)form.get("userInput1_express");
userInput2 = (userInput2 == null) ? "=" : userInput2.replaceAll("'", "''");
if (userInput1 != null && !check.trim().equals("")) {
    iQueryObject.addQuery("in.userInput1", userInput1.replaceAll("'", "''"), userInput2, null);
    sbSelect.append("AND WWXX_YYYYMM " + userInput2 + " '" + userInput1.replaceAll("'", "''") + "' ");
}

So far, we have tried to scan the updated application using BurpSuite and sqlmap, both have been unable to identify any SQL injection issues.

The back-end database being used is Oracle.

Anders
  • 64,406
  • 24
  • 178
  • 215
b.k
  • 3
  • 2
  • 3
    The flaws of this method are addressed in [this answer](https://security.stackexchange.com/a/37751/37315) to [No single quotes is allowed, Is this SQL Injection point still exploitable?](https://security.stackexchange.com/questions/37749/no-single-quotes-is-allowed-is-this-sql-injection-point-still-exploitable). In short, it could be exploitable depending on the database backend. Apart from that, don't just fix a broken design by making it slightly less broken. Instead use the right design, i.e. statements with parameters. See [bobby tables](https://bobby-tables.com/java) for more on this. – Steffen Ullrich Jan 30 '20 at 15:00
  • 1
    Also at stackoverflow.com there is a highly voted question which asks exactly this too: [Can I protect against SQL injection by escaping single-quote and surrounding user input with single-quotes?](https://stackoverflow.com/questions/139199/can-i-protect-against-sql-injection-by-escaping-single-quote-and-surrounding-use). – Steffen Ullrich Jan 30 '20 at 15:10
  • thanks! ill have a look – b.k Jan 30 '20 at 15:14

1 Answers1

3

Yes, your filtering on userinput2 is insufficient, the use of a single quote is not necessary for exploitation with that syntax. I didn't analyse the code any further. "Blacklisting" characters is not a reliable defense against SQL injection. You should alter the code to use parameterized queries instead.

wireghoul
  • 5,745
  • 2
  • 17
  • 26
  • Slightly more specifically: since userInput2 *isn't inside a string to start with*, there's no reason the attacker would need to use a `'` to get out of the string that they aren't in. – user253751 Jan 30 '20 at 18:16