ASP - SQL injection protection in the SELECT clause

Question

After getting great help in securing against SQL injection from classic ASP protection against SQL injection, I've encountered a major issue which cannot be solved using parameterized queries.

name = Trim(Request.QueryString("name"))
flds = Trim(Request.QueryString("flds"))
sql = "set rowcount 0 select " & flds & " from [TABLE] where Name = '" & name & "'"

From what I understand, a parameterized query will protect against SQL injection in the WHERE clause (in this case, the name field.

flds is a comma-separated list of parameters that the users wants returned. As it is obvious, it is very vulnerable to SQL injection.

One idea I have to secure my code is to have a statically generated dict of valid fields, split the flds string by ",", verify each one of the values against the dict, and construct the SQL query that will consist of all the fields that are present in the dict.

It seems to me that although this method will work for security, it will require me to modify the static list at every change in the database (however rare those are).

Are there better/proper ways of securing this code against SQL injection attacks?

Are users really typing in column names? I really hope you're providing them with a list and they're picking, not typing. — Aaron Bertrand, Jun 13 '12 at 19:10
they have a list provided in the documentation. They can select as many as they want, but the users ARE typing in the fields. (this is a web service, so many users connect to it via an API, but that still leaves the security issue) — iliaden, Jun 13 '12 at 19:12

score 2 · Answer 1 · answered Jun 13 '12 at 19:22

Create a split function in SQL Server (there are better ones for newer versions, but this is what you get in SQL Server 2000):

CREATE FUNCTION dbo.SplitStrings
(
   @List       NVARCHAR(4000),
   @Delimiter  CHAR(1)
)
RETURNS @Items TABLE
(
   Item NVARCHAR(4000)
)
AS
BEGIN
   DECLARE
       @Item VARCHAR(12),
       @Pos  INT;

   WHILE LEN(@List)>0
   BEGIN
       SET @Pos = CHARINDEX(@Delimiter, @List);

       IF @Pos = 0
           SET @Pos = LEN(@List)+1;

       SET @Item = LEFT(@List, @Pos-1);

       INSERT @Items SELECT LTRIM(RTRIM(@Item));

       SET @List = SUBSTRING(@List, @Pos + LEN(@Delimiter), LEN(@List));

       IF LEN(@List) = 0 BREAK;
   END
   RETURN;
END
GO

Then create a stored procedure:

CREATE PROCEDURE dbo.RunScaryQuery
  @columns NVARCHAR(4000),
  @table   NVARCHAR(255)
AS
BEGIN
  SET NOCOUNT ON;

  DECLARE @collist NVARCHAR(4000), @sql NVARCHAR(4000);

  SELECT @collist = COALESCE(@collist + ',', '') + c.name 
    FROM syscolumns AS c
    INNER JOIN dbo.SplitStrings(@columns, ',') AS s
    ON s.Item = c.name
    WHERE c.id = OBJECT_ID(@table);

  SELECT @sql = 'SELECT ' + @collist + ' FROM ' + @table
  -- where ...
  ;

  EXEC sp_executesql @sql;
END
GO

Now call that stored procedure from ASP with a properly parameterized command object.

This will ensure that your SQL query is generated only using column names that actually exist in the table. (Any nonsense will be ignored.)

This presumes that you will get at least one valid column name in the list.

+1 This answer gets my vote but the questioners dictionary approach has some merit too. — AnthonyWJones, Jun 13 '12 at 20:00

score 0 · Answer 2 · answered Jun 13 '12 at 19:33

I'm at home, no db to test but this should do it Basically, get all the fields from the db that fit the where, get the requested fields in an array and compare the two lists, putting out only the requested fields.

name = Trim(Request.QueryString("name"))
flds = split(Trim(Request.QueryString("flds")),",")
sql = "set rowcount 0 select * from [TABLE] where Name = '" & name & "'"
set oRst = oConn.execute(sql)
on error resume next
do while not oRst.eof
  result    = ""
  separator = ""
  for each field in flds
    for each requested_field in flds
      if uCase(field.name) = uCase(trim(requested_field)) then
        result = result & separator & field.value
        separator = ","
      end if
    next
  next
  response.write result & "<br>"
  oRst.movenext
loop

This will force the DB to spew out too much data (select *) since there are dozens of fields, and 95% of the queries request less than 5 fields at once. But this solution will work, thanks — iliaden, Jun 13 '12 at 19:36

score 0 · Answer 3 · answered Jun 13 '12 at 20:06

hm... so I'm going with another solution.

I first have an SQL query that return all the valid fields

select
   tcol.name
from
  sysObjects tobj
  join syscolumns tcol on tobj.id = tcol.id
where
  tobj.xtype = 'U'
  and tobj.name = '[TABLE]'

and then I validate every element as suggested by @peter. All the validated parameters are then used to build the query string, and name is passed as a parameter in the second query.

This seems to minimize the overhead and the strain on the database.

score -1 · Answer 4 · answered Jun 14 '12 at 11:06

-1

Have a look at http://www.userfriendlythinking.com/Blog/BlogDetail.asp?p1=7013&p2=119&p7=3001

which shows usage of parameterized queries.

answered Jun 14 '12 at 11:06

Ravi Vanapalli

9,805
3
33
43

this is unrelated to the question. The question is about how to implement the SELECT clause in a parameterized query - not how to pass parameters. – iliaden Jun 14 '12 at 13:32

ASP - SQL injection protection in the SELECT clause

4 Answers4

Linked