Take the 2-minute tour ×
Information Security Stack Exchange is a question and answer site for Information security professionals. It's 100% free, no registration required.

I've written some code to generate an SQL query in ASP classic. I'm not sure if it's secure or not:

Set adoCon = Server.CreateObject("ADODB.Connection")
adoCon.Open "Provider=Microsoft.Jet.OLEDB.4.0; Data Source=" & Server.MapPath(dbfile)
strSQL = "SELECT * FROM Table WHERE name_field LIKE ?"

Set cmd1 = Server.CreateObject("ADODB.Command") 
cmd1.ActiveConnection = adoCon 
cmd1.CommandText = strSQL 
cmd1.CommandType = 1 'adCmdText 
cmd1.Parameters(0) =  Request("odQuery") & "%"  
Set rs = cmd1.Execute()

I then go on to display the results from rs.

Is this secure? (I know for instance I can put a percent sign at the start of odQuery and make the code return more records).

share|improve this question
add comment

2 Answers 2

You are already using parametrized queries which protects you against injection of SQL parts, assuming you are always using parameters for untrusted data.

As you said that leaves the issue of wildcard characters in operators that support them. That is, if you are using "=" instead of LIKE, the problem will not arise. In most cases, in which LIKE is used intentionally, a search with user supplied data is desired. So adding more wildcards is likely not an issue.

Adding a wildcard at the front, however, may have a significant performance impact because prefix indices cannot be used efficiently. So if your dataset is huge, you may want to filter them. The standard wildcars are % and _, but some databases support additional characters such as *.

share|improve this answer
    
Thanks. I'm mainly concerned about the security aspect. I've never used parametrized queries before and wasn't sure if I was doing it correctly. –  Flash Aug 15 '11 at 7:46
add comment

This is not secure, for several reasons - which you probably are not even considering.

Let's start at the top:

  • You are using ASP "classic". This platform is known to be insecure.
  • Your provider is Microsoft.Jet - I'm assuming this is connecting to an MS Access file... Again, not a secure backend. E.g. no authentication to the database, issues with a shared file, etc.
  • I don't know if you realize this, if it's intentional, or if it even matters in your application, but you're taking the parameter from Request("odQuery") - which could be Request.Forms("odQuery"), Request.QueryString("odQuery"), etc... The attacker can misuse this, e.g. by crafting a link (instead of a POST) and having someone else (e.g. with higher privileges) submit it.
  • I'm speculating here, but judging from the previous code (and the fact that this is "classic ASP"), "I then go on to display the results from rs" I am guessing will highly likely result in XSS (Cross-Site scripting).
  • Now, let's talk about SQL Injection - as @Hendrik pointed out in his answer, wildcards are an issue. This may have 2 problems:
    • Accessing data which the user should not have access to
    • Creating a DoS (Denial of Service) attack, by causing very computational intensive searches, such as putting the wildcard at the front in large datasets (as @Hendrik pointed out).

If you are specifically looking for a way to execute a secondary query, you are safe from this, at least on Access (some benefits from not using a full-featured DB...).
However, there are other forms of SQL Injection (as above), and even more ways of being insecure.

share|improve this answer
1  
Using POST instead of GET is not a security feature because it is very easy to trick someone into submitting a form, if you are already able to trick him to follow a link. The form submission can be done complete automatic with JavaScript. POST should be used when you have a huge amount of data (such as uploaded files) and/or you want to prevent accidental resubmission. GET should be uses for less than 2000 characters in the URL, if resubmitting is not an issue. This is especially true for search result because it allows bookmarking. I guess the code in the question is for a search query. –  Hendrik Brummermann Aug 15 '11 at 8:37
    
@Hendrik, I agree - but I was also specific that we don't really have any much way of knowing if it matters in his context. That said, I think you're probably right that it's a search query, and as such of lower priority. But it is still an issue. –  AviD Aug 15 '11 at 8:50
2  
Btw GET vs POST is not just about size - according to the RFC, GET should only be used if it is idempotent - i.e. if any changes are made, POST should be used. If not, it could be either way. –  AviD Aug 15 '11 at 8:51
2  
It is a very good idea to use POST for any query that have side effects. I think there might be subtle things that can go wrong if you don't do this (e.g., some crazy kinds of CSRF); I can't remember the list, but I'm pretty sure I remember running across some obscure stuff. Using POST may not be a security feature, but using GET for a side-effectful request is likely to be an "insecurity feature". –  D.W. Aug 15 '11 at 20:39
    
@D.W. I agree, though it really depends on context - i.e. whether or not there is a side effect (and, btw, logging and such should apparently be excluded from that rule...) –  AviD Aug 15 '11 at 20:43
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.