Skip to content
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

mssql get_table_name bug #583

Closed
pareeohnos opened this issue Sep 9, 2014 · 4 comments
Closed

mssql get_table_name bug #583

pareeohnos opened this issue Sep 9, 2014 · 4 comments
Labels

Comments

@pareeohnos
Copy link

I'm not sure if this is also an issue for other adapters, but I've only noticed it for the MSSql adapter.

The application I'm working on retrieves application generated alerts about the status of the software. These alerts primarily contain errors, and in some cases these error contain a SQL query that was run that caused the error. Until recently I hadn't had any issues until a particular error arrived that caused the application to fail, stating that the table didn't exist.

Strangely though, the table it was searching for is not a table within the application itself, but rather a table that the remote software is using, so I knew that the error was somehow being injected. I've done some more digging as I thought that it may have been an accidental SQL injection, however all of the SQL that our application builds up uses active record, and everything is escaped.

The error I was getting pointed to the get_table_name method within the utils.rb file of the MSSQL adapter. I looked at the code and saw that regexes are being use to determine the table name from the query being run, and after a quick test realised that this was the problem.

The query that my application is attempting to run is a simple SELECT COUN(*), however the WHERE clause contains an INSERT statement. An example of a failing query is

SELECT COUNT(*)
FROM our_table
WHERE text = "
INSERT INTO their_table
VALUES ('a', 'b', 'c')
"

The MSSQL adapter then uses the regex

^\s*(INSERT|EXEC sp_executesql N'INSERT)\s+INTO\s+([^\(\s,]+)\s*|^\s*update\s+([^\(\s,]+)\s*

to pull the table names. If you run this regex against this query on something like regexpal.com with the m flag enabled, it highlights the INSERT INTO their_table instead of the FROM our_table.

I THINK this would be resolved by simply changing the order of the if statement to run the other regex first, however this could well cause issues for other queries.

This can also easily be replicated within rails by simply doing something like

Model.where(field: "\nINSERT INTO table")

Which gives the stack trace

ActiveRecord::JDBCError: table: test' does not exist
from arjdbc/jdbc/RubyJdbcConnection.java:1098:in `columns'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/jdbc/adapter.rb:322:in `columns'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/mssql/adapter.rb:540:in `columns'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/mssql/adapter.rb:704:in `special_column_names'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/mssql/adapter.rb:692:in `repair_special_columns'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/mssql/adapter.rb:620:in `exec_query'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-jdbc-adapter-1.3.9/lib/arjdbc/jdbc/adapter.rb:518:in `select'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/connection_adapters/abstract/database_statements.rb:31:in `select_all'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/connection_adapters/abstract/query_cache.rb:69:in `select_all'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/relation/calculations.rb:254:in `execute_simple_calculation'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/relation/calculations.rb:216:in `perform_calculation'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/relation/calculations.rb:111:in `calculate'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/activerecord-4.1.1/lib/active_record/relation/calculations.rb:26:in `count'
from (irb):4:in `evaluate'
from org/jruby/RubyKernel.java:1101:in `eval'
from org/jruby/RubyKernel.java:1501:in `loop'
from org/jruby/RubyKernel.java:1264:in `catch'
from org/jruby/RubyKernel.java:1264:in `catch'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/railties-4.1.1/lib/rails/commands/console.rb:90:in `start'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/railties-4.1.1/lib/rails/commands/console.rb:9:in `start'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/railties-4.1.1/lib/rails/commands/commands_tasks.rb:69:in `console'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/railties-4.1.1/lib/rails/commands/commands_tasks.rb:40:in `run_command!'
from C:/JRuby/jruby-1.7.13/lib/ruby/gems/shared/gems/railties-4.1.1/lib/rails/commands.rb:17:in `(root)'
from org/jruby/RubyKernel.java:1065:in `require'
from bin/rails:4:in `(root)'

Running the same code in rails with a postgres connection works as expected.
I'm working with Rails 4.1.1, jRuby 1.7.13, and activerecord-jdbc 1.3.9

you can support MS-SQL fixes at BountySource

@kares
Copy link
Member

kares commented Sep 9, 2014

please always confirm the issue on latest release ... in this case 1.3.10 - it's probably there but we shall make sure (traces might differ). there's lot of excellent explanation but a (pseudo) piece of code reproducing the issue would be great.

AR-JDBC and esp. the MS-SQL adapter is mostly as good as the community of JRuby @ SQLServer users make it, just in case you'll be wating for the fix :)

@kares kares added the mssql label Sep 9, 2014
@pareeohnos
Copy link
Author

Apologies, hadn't noticed a new version so hadn't updated but I'll try it at some point. The code I was looking through however was from github and not my local copy so any code references here should be from the latest version.

Not in a major hurry for a fix so no worries. Just in case anyone else runs into it :)
I can work around it by simply indenting the INSERT statement as the regex is searching for the beginning of a line only.

For replicating it, not sure what to put for pseudocode or actual code. You can replicate it with a 1 liner in any existing rails project. I can edit the original post to include an example with migrations and model creation if needed? Otherwise the example that's already there will do it

Model.where(field: "\nINSERT INTO")

:)

@kares
Copy link
Member

kares commented Sep 9, 2014

@pareeohnos thanks a lot ... I thought that there was some special table naming necessary - my bad!

@kares
Copy link
Member

kares commented Sep 9, 2015

😋 if we omit the year ... we can call it a same day fix :)

@kares kares closed this as completed Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants