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.

  • 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)" – All Blond Apr 24 '14 at 21:01
  • here is some good material about user defined table types and how you can use those: http://www.codeproject.com/Tips/93248/SQL-Server-User-Defined-Table-Types-and-Table – All Blond Apr 24 '14 at 21:13
  • @AllBlond we use SQL Server 2008 – Virginia Norman Apr 25 '14 at 12:20

5 Answers5

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
Bond
  • 16,071
  • 6
  • 30
  • 53
  • 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. – Virginia Norman Apr 24 '14 at 17:53
  • 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. – Virginia Norman Apr 24 '14 at 18:21
  • What's the data type of column `studentID` in the `students` table? – Bond Apr 24 '14 at 18:30
  • @Bond Sorry but those constants are mapped wrong, see [this very useful guide](http://www.carlprothman.net/Technology/DataTypeMapping/tabid/97/Default.aspx) I've used time and again. – user692942 Apr 24 '14 at 18:36
  • @Lankymart Your guide uses the same constants as mine? Which ones are mapped wrong? – Bond Apr 24 '14 at 18:41
  • I think the issue here might be that `studentID` was created as data type `text` in the `student` table. And `text` is a deprecated data type. But we won't know for sure until @WhoKnewNewbie responds. – Bond Apr 24 '14 at 18:56
  • studentID's data type is varchar(35) – Virginia Norman Apr 24 '14 at 18:57
  • Hmmm. I wonder why we're getting the "varchar and text are incompatible" message. `adBSTR` should be compatible with `varchar` so it seems like the `text` or `ntext` type is coming into play here. But let's try using `varchar` specifically. Can you replace `vbString` with `200` (the value of adVarChar) in my code above and see if that works? – Bond Apr 24 '14 at 18:59
  • There's some _great_ info here, but this ultimately won't work, because you need a table-valued parameter for an IN() condition, which is not supported in classic asp. – Joel Coehoorn Apr 25 '14 at 00:46
  • @JoelCoehoorn Not supported in classic ASP or not supported in classic ADO? – Bond Apr 25 '14 at 00:58
  • Yes, I should say ADO, though it's kind of hard to separate the two. – Joel Coehoorn Apr 25 '14 at 01:50
  • @JoelCoehoorn Not sure I've misunderstood but as long as you [match the number of parameters](https://stackoverflow.com/a/61042739/692942) into the `IN` statement it will work fine. – user692942 Apr 05 '20 at 12:45
  • @Bond The problem with those data type mappings is you're mapping VBScript data-types to ADO not ADO to the ADO provider. For example a string in SQL Server can be `VARCHAR`, `NVARCHAR`, `VARCHAR(MAX)` and `NVARCHAR(MAX)` which all map to different data types which I mention in [my previous comment](https://stackoverflow.com/questions/23274891/pass-a-vbscript-string-list-to-a-sql-inoperator?noredirect=1&lq=1#comment35626303_23275255) from 6 years ago. – user692942 Apr 05 '20 at 12:57
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 = ?
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
  • 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. – All Blond Apr 24 '14 at 21:53
  • Reading your article today and hoping to resolve this issue today. Will update this with the solution if one is found. – Virginia Norman Apr 25 '14 at 12:22
  • 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 http://stackoverflow.com/a/337752/438107 – David Storfer Apr 16 '15 at 17:36
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.

user692942
  • 16,398
  • 7
  • 76
  • 175
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
-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.

All Blond
  • 802
  • 1
  • 8
  • 16
  • 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. – Virginia Norman Apr 25 '14 at 12:01
  • @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? – All Blond Apr 25 '14 at 18:26
  • @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. – All Blond Apr 25 '14 at 18:33
  • 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. – Virginia Norman Apr 25 '14 at 18:50
  • 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. – All Blond Apr 25 '14 at 19:25