0

I am writing a query to allow a user to search on what they provide keywords in asp.net, C# and mssql:

 string projectPart = null;
 string categoryPart = null;
 string descriptionPart = null;

 if (this.Textbox_ProjectNr.Text.Trim().Length > 0)
   projectPart = " AND Number='" + this.Textbox_ProjectNr.Text.Trim() + "' ";

 if (this.Textbox_Category.Text.Trim().Length > 0)
   categoryPart = " AND Category LIKE '%" + this.Textbox_Category.Text.Trim() + "%' ";

 if (this.Textbox_pDescription.Text.Trim().Length > 0)
    descriptionPart = " AND ProductDescription LIKE '%" + this.Textbox_pDescription.Text.Trim() + "%' ";

 string query = "SELECT * from Project  = p.ID " + projectPart + descriptionPart + categoryPart;

I dont know whether this query is sufficient for a traditional query search. Because I see there are some bottlenecks of this search:

  1. if the user does not type anything, it returns all of the data => For this I only do the query when one of the fields are filled.
  2. if the user provides some keywords "P" for each field, the result will be millions of data.

I dont know how to improve the search query basically. any suggestions are appreciated.

Thanks in adavance.

olidev
  • 20,058
  • 51
  • 133
  • 197
  • and do you want it to return millions of data? – aF. Nov 18 '11 at 09:55
  • As other users have already mentioned injection attacks this may also be worth a read, it says why select * is generally a bad idea http://stackoverflow.com/questions/321299/what-is-the-reason-not-to-use-select – Purplegoldfish Nov 18 '11 at 10:11

3 Answers3

3

The most important improvement is to protect you code against SQL injection attacks.

You should not concatenate the raw input in the SQL string. If someone searches for the following text for example:

Bwah ha ha'; DROP DATABASE northwind; PRINT'

This will be added to your query to produce

SELECT *
FROM mytable
WHERE category LIKE '%Bwah ha ha'; DROP DATABASE northwind; PRINT'%'

This is a valid SQL command and will happily execute and drop your database (or do anything else the attacker wants)

For more information see SQL Injection and Santitizng Inputs.

njr101
  • 9,499
  • 7
  • 39
  • 56
1

You must make this query injection proof! Do not concatenate user entered values, but use parameters, like this:

SqlCommand cmd = new SqlCommand(@"
    SELECT * from Project 
    WHERE 
    ( Number = @Number OR @Number IS NULL ) AND
    ( Category LIKE @Category OR @Category IS NULL ) AND
    ( ProductDescription LIKE @ProductDescription OR @ProductDescription IS NULL )", conn);
if(!String.IsNullOrEmpty(this.Textbox_ProjectNr.Text.Trim()))
   cmd.Parameters.AddWithValue("@Number", this.Textbox_ProjectNr.Text.Trim());
if(!String.IsNullOrEmpty(this.Textbox_Category.Text.Trim()))
   cmd.Parameters.AddWithValue("@Category", this.Textbox_Category.Text.Trim());
if(!String.IsNullOrEmpty(this.Textbox_pDescription.Text.Trim()))
   cmd.Parameters.AddWithValue("@ProductDescription", this.Textbox_pDescription.Text.Trim());

Also, you can add some client validation on user entered values. For instance, ask for more than three (?) characaters before running that query.

<asp:TextBox ID="Textbox_ProjectNr" runat="server" />
<asp:RegularExpressionValidator ID="Textbox_ProjectNr_Validator" runat="server" 
   ControlToValidate="Textbox_ProjectNr"
   ErrorMessage="Minimum length is 3"
   ValidationExpression=".{3,}" />
Rubens Farias
  • 57,174
  • 8
  • 131
  • 162
0

First of all, you must protect yourself from sql injections. You haven't specified what connection to the database you are using but most libraries allow adding the parameters in a different field, so they are sanitized automatically. Secondly, you can (and should) limit the results count using the "LIMIT" (for mysql) or "TOP X" Like so: Select * from TableName LIMIT 100 or Select TOP 100 * from TableName

idanzalz
  • 1,740
  • 1
  • 11
  • 18