1

In the vb script I have a select statement I am trying to pass a string value with an undetermined length to a SQL in operator the below code works but allows for SQL injection.

I am looking for a way to use the ADO createParameter method. I believe the different ways I have tried are getting caught up in my data type (adVarChar, adLongChar, adLongWChar)

    Dim studentid 
studentid = GetRequestParam("studentid")

Dim rsGetData, dbCommand
    Set dbCommand = Server.CreateObject("ADODB.Command")
    Set rsGetData = Server.CreateObject("ADODB.Recordset")
    dbCommand.CommandType = adCmdText
    dbCommand.ActiveConnection = dbConn
dbCommand.CommandText = "SELECT * FROM students WHERE studentID in (" & studentid & ")"
Set rsGetData = dbCommand.Execute()

I have tried

Call addParameter(dbCommand, "studentID", adVarChar, adParamInput, Nothing, studentid)

which gives me this error ADODB.Parameters error '800a0e7c' Problems adding parameter (studentID)=('SID0001','SID0010') :Parameter object is improperly defined. Inconsistent or incomplete information was provided.

I have also tried

Call addParameter(dbCommand, "studentID", adLongVarChar, adParamInput, Nothing, studentid)

and

    Dim studentid 
studentid = GetRequestParam("studentid")

Dim slength
slength = Len(studentid)
response.write(slength)

Dim rsGetData, dbCommand
    Set dbCommand = Server.CreateObject("ADODB.Command")
    Set rsGetData = Server.CreateObject("ADODB.Recordset")
    dbCommand.CommandType = adCmdText
    dbCommand.ActiveConnection = dbConn
dbCommand.CommandText = "SELECT * FROM students WHERE studentID in (?)"
    Call addParameter(dbCommand, "studentID", adVarChar, adParamInput, slength, studentid)
Set rsGetData = dbCommand.Execute()

both of these options don't do anything... no error message and the SQL is not executed.

Additional information:

studentid is being inputted through a HTML form textarea. the design is to be able to have a user input a list of student id's (up to 1000 lines) and perform actions on these student profiles. in my javascript on the previous asp I have a function that takes the list and changes it into a comma delimited list with '' around each element in that list.

Improve this question
3
  • What database are you using? If it is SQL Server 2005 or better then create user defined Table type (even express edition has it), pass to it your studentID and then you can do your normal "...where id in (select StudentId from YourType)" Commented Apr 24, 2014 at 21:01
  • here is some good material about user defined table types and how you can use those: codeproject.com/Tips/93248/… Commented Apr 24, 2014 at 21:13
  • @AllBlond we use SQL Server 2008 Commented Apr 25, 2014 at 12:20

5 Answers 5

Reset to default
2

Classic ASP does not have good support for this. You need to fall back to one of the alternatives discussed here:

http://www.sommarskog.se/arrays-in-sql-2005.html

That article is kind of long, but in a good way: it's considered by many to be the standard work on this subject.

It also just so happens that my preferred option is not included in that article. What I like to do is use a holding table for each individual item in the list, such that each item uses an ajax request to insert or remove it from the holding table the moment the user selects or de-selects it. Then I join to that table for my list, so that you end up with something like this:

SELECT s.* 
FROM students s
INNER JOIN studentSelections ss on s.StudentID = ss.StudentID
WHERE ss.SessionKey = ?
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Basically what Joel recommend is exactly the same as I did in my comments: use user defined table types and Store procedures, but it will work only if you are on SQL server 2005 or better, even express edition.
Reading your article today and hoping to resolve this issue today. Will update this with the solution if one is found.
I prefer to use a table-valued user-defined function which parses a comma-delimited (or any delim) list into a table, which you can easily select in the IN clause. This is described in stackoverflow.com/a/337752/438107
2

What does your addParameter() function do? I don't see that anywhere in your code.

You should be able to create and add your string param like so:

With dbCommand
    .Parameters.Append .CreateParameter(, vbString, , Len(studentid), studentid)
End With

(Small hack here. vbString has the same value as adBSTR. You'll find that the VarType of all VB "types" have matching ADO counterparts.)

Type       VarType (VBScript)  DataTypeEnum (ADO)  Value
---------  ------------------  ------------------  -----
Integer    vbInteger           adSmallInt, 2-byte      2
Long       vbLong              adInteger, 4-byte       3
Single     vbSingle            adSingle                4
Double     vbDouble            adDouble                5
Currency   vbCurrency          adCurrency              6
Date       vbDate              adDate                  7
String     vbString            adBSTR                  8
Object     vbObject            adIDispatch             9
Error      vbError             adError                10
Boolean    vbBoolean           adBoolean              11
Variant    vbVariant           adVariant              12
Byte       vbByte              adUnsignedTinyInt      17

Edit: Looks like Joel has a good solution for you. I didn't realize IN isn't compatible with ADO parameterized queries. I think something like the following would work, but you probably wouldn't want to do it with (potentially) 1000 ID's.

' Create array from student IDs entered...
a = Split(studentid, ",")

' Construct string containing proper number of param placeholders. Remove final comma.
strParams = Replace(String(UBound(a) - 1, "?"), "?", "?,")
strParams = Left(strParams, Len(strParams) - 1)

With dbCommand
    .CommandText = "select * from students where studentID in (" & strParams & ")"
    Set rsGetData = .Execute(, a)
End With
Improve this answer

13 Comments

The addParameter() is a function used to clean up the values that would be used by the ADO method .CreateParameter(name,type,direction,size,value). essentially they are the same we just have some error handling in the addParameter() function.
I tried vbString and adBSTR and recieved this error: Microsoft OLE DB Provider for SQL Server error '80040e14' The data types varchar and text are incompatible in the equal to operator.
What's the data type of column studentID in the students table?
@Bond Sorry but those constants are mapped wrong, see this very useful guide I've used time and again.
@Lankymart Your guide uses the same constants as mine? Which ones are mapped wrong?
|
1

Using IN with a Parameterised Query isn't Hard

Posting this here in relation to another question that was marked as a duplicate of this one.

This isn't as difficult as you think, the adCmdText query just needs to the placeholders in the query to match the number of parameters and their ordinal position and it will work with any number of parameters you pass into an IN statement.

Here is a quick example using the AdventureWorks example database in SQL Server. We use an Array to store the id of each Person.Contact record we wish to filter out using IN than build the parameters dynamically based on that array before executing the ADODB.Command.

Note: The source of the array is not important it could be a string list which is Split() into an Array or just an Array() call (like the one used in this example).

<%
Option explicit
%>
<!-- #include virtual = "/config/data.asp" -->
<%
Dim cmd, rs, sql, data, rows, row
'Our example parameters as an Array for the IN statement.
Dim ids: ids = Array(2, 5, 10)
Dim id

sql = ""
sql = sql & "SELECT [FirstName], [LastName] " & vbCrLf
sql = sql & "FROM Person.Contact " & vbCrLf
sql = sql & "WHERE [ContactId] IN (?, ?, ?) " & vbCrLf
sql = sql & "ORDER BY [LastName], [FirstName]" & vbCrLf

Set cmd = Server.CreateObject("ADODB.Command")
With cmd
  .ActiveConnection = conn_string
  .CommandType = adCmdText
  .CommandText = SQL
  'Loop through the Array and append the required parameters.
  For Each id in ids
    Call .Parameters.Append(.CreateParameter("@id" & id, adInteger, adParamInput, 4))
    .Parameters("@id" & id).Value = id
  Next
  Set rs = .Execute()
  'Output the Recordset to a 2-Dimensional Array
  If Not rs.EOF Then data = rs.GetRows()
  Call rs.Close()
End With
Set cmd = Nothing

If IsArray(data) Then
  rows = UBound(data, 2)
  For row = 0 To rows
    Call Response.Write("<p>" & data(0, row) & " " & data(1, row) & "</p>" & vbCrLf)
  Next
End If
%>

Output:

<p>Catherine Abel</p>
<p>Pilar Ackerman</p>
<p>Ronald Adina</p>

Worth noting this example shows the explicit way of writing the parameter code for an elegant approach to the problem see the second solution in @Bond's answer.

Improve this answer

Comments

0

After reading through the article that was provided by Joel and the answer that All Blond provided this is the solution that ended up working for me. 

Dim studentid
studentid = GetRequestParam("studentid")

Dim splitStudentid, x
splitStudentid = Split(studentid,",")

for x=0 to Ubound(splitStudentid)
Dim rsGetData, dbCommand, originSID
Set dbCommand = Server.CreateObject("ADODB.Command")
Set rsGetData = Server.CreateObject("ADODB.Recordset")
dbCommand.CommandType = adCmdText
dbCommand.ActiveConnection = dbConn
dbCommand.CommandText = "SELECT * FROM students WHERE studentID=?"
Call addParameter(dbCommand, "studentID", adVarChar, adParamInput, 35, splitStudentid(x))
Set rsGetData = dbCommand.Execute()

If (NOT rsGetData.EOF) Then 
    originSID = rsGetData.Fields(0)
    //additional code
End If
next

I found that there was no elegant way to use the "in" operator in my code. I also decided against a Stored Procedure as it is a simple query though I agree

ALSO I realize that addParameter is a Function my company uses internally so below is an additional solution that works also works but is not my companies preference. 

    Dim studentid
studentid = GetRequestParam("studentid")

Dim splitStudentid, x
splitStudentid = Split(studentid,",")

for x=0 to Ubound(splitStudentid)
Dim rsGetData, dbCommand, originSID
Set dbCommand = Server.CreateObject("ADODB.Command")
Set rsGetData = Server.CreateObject("ADODB.Recordset")
dbCommand.CommandType = adCmdText
dbCommand.ActiveConnection = dbConn
dbCommand.CommandText = "SELECT * FROM students WHERE studentID=?"
Set rsGetData = dbCommand.Execute(, Array(splitStudentid(x)))

If (NOT rsGetData.EOF) Then 
    originSID = rsGetData.Fields(0)
    //additional code
End If
next
Improve this answer

Comments

-1

Try to add to your code following(assuming that StudentID is numeric)

 dim outputArray,x,compareArray
 outputArray=split(inputText,",")
 for each x in outputArray
  If IsNumeric(x) Then
      if len(compareArray)>1  and cInt(x)>0 then
         compareArray=compareArray&"," & cInt(x)
      else
          compareArray=cInt(x)
      end if
   else
     ' throw some log entry or do something for you to know that someone try
   end if
 next

and then do all your Db connection set etc up to this point where you use this new string of integers:

  dbCommand.CommandText = "SELECT * FROM students WHERE studentID in (" & compareArray &")"

and this will safeguard you from anyone use your StudentId list for SQL injection. I would rather use Store procedure and user-defined table types but ...

In any case if it is not numeric then it must have some parameter like length or complexity which you can use to verify that value has not been compromised using regular expression for example for limiting what can be in that value; but idea of the looping through and verifying values remain the same.

Improve this answer

5 Comments

This is actually the most elegant way I have seen someone use the for loop to create an array I will try something like this. Side notes: The Student ID is non numeric and can vary in length depending on the origination of the student. examples: SID12345, S-100100100. I will try to use the student origination some how to determine length, try your solution, and let you know if this worked for me.
@WhoKnewNewbie since you are on SQL 2008 use Store procedure and you would not need to do loop, just send data to user-defined table data types. It will improve performance of your application and also helps with fight against SQL injection. And all that could be done within Store procedure itself! And as a side-note- could you do something to uniform Student id for example to the same length and format or this is out of your jurisdiction?
@WhoKnewNewbie Explain to the business owners that it will improve performance and security of database, which in terms will decrease expenses. Usually those arguments weight heavily on business especially when they will see it at the first run of your query. Instead of "in" you would use "where id=id" - search by direct indexed column will replace heavy table scan on each record! This will be true even without uniformed StudentID.
The company is fairly small. They are working on a few overhauls to the database and they know the issues. At this time I am a lowly intern and have very little influence. I make coffee and do the coding projects with no deadlines.
Show them that a lowly intern have a brain and not afraid to use it (lol) how else would they know that you exists? Just formulate and ask a question to show that you know something which they may not consider just because they been on it for too long.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.