0

I've made some posts about translating user input from a search box into an ActiveRecord Parameterized statement, which may be vulnerable to SQL injection, but I think I may have a solution.

Here's how the search works. The user enters something like this into the search box:

name="hello" AND NOT address.town="Villa"

Internally, I convert it to:

query = ["name LIKE ? AND address.town NOT LIKE ?", "hello", "villa"]

for:

if search
  query = convert_search_to_query search
  begin
    includes(:address).where(query)
  # rescue Exception ...
...
...

Here's my idea: simply check the user-inputted attributes ("name", "address.town" in this case) to make sure it's an exact match for the acceptable list of user attributes.

If I were to do this, I think that there would be no SQL Injection possible since I am using parameterized statements (with the '?') to handle the only part of the user's input I can't check -- the values he entered for each attribute.

Based on what I read from other posts on here, I don't see how this code could be any more vulnerable than a normal parameterized search, but I don't have a lot of experience with SQL injection. Is it vulnerable?

Also:

I understand that there are plugins that may be able to help, but what I want to do is really very simple, and is already working, and I'd rather keep my app as lightweight as possible.

Community
  • 1
  • 1
ineedahero
  • 488
  • 2
  • 7
  • 22
  • 2
    To prevent injection in rails, just use Active Record. There's no need to ever have a direct SQL query in rails. – Eli Sadoff Oct 31 '16 at 13:05
  • Would that work when the user could enter a dynamic number of queries? – ineedahero Oct 31 '16 at 13:09
  • 1
    ActiveRecord works anywhere SQL works. Read its documentation. It makes life a lot easier than using SQL and having to worry about injection. – Eli Sadoff Oct 31 '16 at 13:10
  • Oh. Sorry for not explaining my question correctly. I am using ActiveRecord, in a format similar to the following: Client.where("orders_count = ?", params[:orders]) – ineedahero Oct 31 '16 at 13:12
  • 1
    It would all depend on how you convert the user input into parameters. If that process contains any flaws or vulnerabilities then you will be exposed to injection attacks. There are companies that can test your sites for you (penetration testing). Security is a big & complex subject. For that reason, I avoid rolling my own solutions. – David Rushton Oct 31 '16 at 13:29
  • Thanks! One last followup -- in the worst event, what would be the maximum damage an SQL injection could do? Since this is contained within a 'where' clause, is the system merely vulnerable to data-read injections? Could deletions or updates be possible? Or does it depend? – ineedahero Oct 31 '16 at 13:52
  • Unfortunately even in a where clause, the maximum damage is bad. The classic example is to inject a close quote and start a new statement, like: `'; DROP TABLE users; --`. When injected into a where clause, this becomes `where foo = ''; DROP TABLE USERS; --'...` (see https://xkcd.com/327/) – gmcnaughton Oct 31 '16 at 14:17

1 Answers1

1

ActiveRecord will prevent any SQL injection attacks, AS LONG AS you are using the parameterized form. As a rule of thumb, ALL information coming from the user should be a parameter.

In your example you mention converting the user query into:

where(["name LIKE ? AND address.town NOT LIKE ?", "hello", "villa"])

In this case ActiveRecord will protect hello and villa from SQL injection, but it will NOT protect name LIKE ? AND address.town NOT LIKE ?. ActiveRecord assumes that name LIKE ? AND address.town NOT LIKE ? is being generated either by the developer (hard coded) or by the application, either way it assumes it's safe to execute.

So if any part of name LIKE ? AND address.town NOT LIKE ? is coming from the user your app could be vulnerable to SQL injection attacks.

The proper way to do it would be to use a language parser to completely decompose the user query and then re-generate it as a safe query. Using Regex to match and replace could be a naive approach unless you are a master in Regex and security.

guzart
  • 3,700
  • 28
  • 23