1

Is this a safe way to filter strings into an SQL query in java or could this be bypassed? I understand that I should just parameterize everything but this is an ancient code base where it makes more sense to do it myself.

    public static String escapeSQL(String input)
    {
        if (input == null)
            return null;

        return input.replaceAll("'", "\\\\'");
    }
  • 3
    What happends if I send in a string with \'? I guess it would turn into \\\' where the backslash is escaped, and not the single quote? I understand that dealing with old code is a pain, but it is very very hard to get SQLi prevention right without parametrization. – Anders Jan 23 at 12:50
  • 5
    Why does it make more sense to use a homebrew solution than a tried-and-true method? – MechMK1 Jan 23 at 12:54
4

This isn't even remotely close to safe. There are any number of gotchas:

  1. As pointed out by Anders in a comment, this can be easily bypassed by injecting in \'. This works because it escapes your backslash, turning it into a literal backslash and causing the single quote to become unescaped. Moreover, this is actually a very common thing to attempt during penetration testing - only escaping single quotes is a common mistake, so this is one of the first things I would try as an attacker.
  2. You are dealing with escaping strings, but integer input is more dangerous. For a query like this: SELECT * FROM table WHERE id=[input], the user input isn't quoted with strings anyway, so an attacker can inject a payload without requiring a single quote, completely escaping your security. Of course you would defeat this by ensuring that any input used in a condition like the above is actually an integer, but what if you forget to do that somewhere? What if someone changes the clause to make it an integer but forgets to change the escaping rules? Stuff like that happens all the time, and it is very easy for code that was secure to become insecure accidentally, without anyone noticing. The only guard against that is prepared queries.
  3. You've properly accounted for multi-byte characters and differences in character encoding, right? Because those two edge cases have bitten a number of systems that attempted the simple escaping that you are doing (here is a random example).

There are a few more gotchas, but those are the biggest. #1 is a deal breaker - your chosen implementation is not at all secure. #2 is also extremely dangerous and should highlight how easy it is to accidentally introduce SQL injection vulnerabilities if you don't use prepared queries. So, if you will forgive me for being blunt: you're attempting to secure a system but have missed some critical and well-known gotchas. That suggests that you might not yet have the experience to come up with your own custom solution. That's really no big deal though - we're all learning, and we all had to figure this out at some point in time. Still, it puts things in perspective and suggests one of two options:

  1. You can continue to try to secure this with somewhat hacky solutions. Now you know that these things are more complicated than you might have originally hoped, so if you go down this path you do so with the knowledge that the risk of an SQL injection vulnerability is high.
  2. Or you can figure out how to get prepared queries in there and be much more safe.

The choice is yours!

|improve this answer|||||
  • Thank you. This is actually not my code, I found this in an open source project and thought it was bad. I just wasn't having any luck trying to exploit it and wanted to make sure I wasn't missing anything. – Muse4665 Jan 23 at 14:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.