New Community Website

Ordinarily, you'd be at the right spot, but we've recently launched a brand new community website... For the community, by the community.

Yay... Take Me to the Community!

Welcome to the DNN Community Forums, your preferred source of online community support for all things related to DNN.
In order to participate you must be a registered DNNizen

HomeHomeDNN Open Source...DNN Open Source...Module ForumsModule ForumsReportsReportsdnn Reports updated to VSE 2008 + token supportdnn Reports updated to VSE 2008 + token support
Previous
 
Next
New Post
9/11/2009 12:25 PM
 

Anytime you introduce data the user could have entered directly into a SQL query, you must be aware of the possibility of attack.  For example, if you use a user's City profile property in a query

SELECT * FROM mystore_Locations WHERE City = '[User:City]'

If the "City" profile property is not correctly sanitized, a user could enter the following as their City:

'; UPDATE dnn_Users SET IsSuperUser = 1 WHERE UserId = 1234; SELECT '

Which would change the script above to:

SELECT * FROM mystore_Locations WHERE City = ''; UPDATE dnn_Users SET IsSuperUser = 1 WHERE UserId = 1234; SELECT ''

(I'm working somewhat from memory, so the exact database fields may not be correct, but I hope you get the idea).

The point is not that TokenReplace is accessing the database, it's that you could be introducing user editable fields directly into the SQL query used by the Reports Module.  Even if TokenReplace or the Reports Module itself did some checking to remove SQL commands from those strings, I would not feel comfortable using it as there are just so many different SQL Injection attack vectors.  Especially not when SQL Server has a parameter model designed explicitly to avoid this problem.

As I said, it all depends on what data you are retrieving from TokenReplace, which is why I don't have a problem with people making the change themselves, but they must be aware of the potential problems if they use data entered by regular users.


Andrew Nurse
DotNetNuke Core Team Member and Reports Module Project Lead
Microsoft Certified Professional Developer

 
New Post
9/13/2009 3:04 AM
 

Andrew, thanx for very nice example of 'injection'.... anyway I still believe that tokens are mandatory for report module. Why dont U simple replace your current warning message in module settings that 'sql might cause .... blah blah' with above example ?. Anybody who plans to use report module is 100% able to write sql query = will very quickly understand what do U mean above...

A.

 
New Post
9/13/2009 1:17 PM
 

I absolutely understand the need for parameterizing queries based on data from DotNetNuke objects, and I intend to implement such a feature.  Tokens aren't the secure way to do that, but I have designed a system that has the same power as Tokens but with full security against SQL Injection (using SQL Parameterized Queries).  As I said, I just haven't had time to implement it.  It will be part of the next major release of the module, I just can't guarantee when that will be.


Andrew Nurse
DotNetNuke Core Team Member and Reports Module Project Lead
Microsoft Certified Professional Developer

 
New Post
9/13/2009 4:24 PM
 

hm,... thinking about parametrized SQL....

who will disable then to do this:

SELECT * FROM mystore_Locations WHERE User = @Portal_user_name

where @Portal_user_name  = ''; UPDATE dnn_Users SET IsSuperUser = 1 WHERE UserId = 1234; SELECT ''

I cant see difference comparing to current implementation, except that parameters are more comfortable for programming.

By filter/check parameters U will limit users creativity and module possibilities. I believe that U can have either secure module but with limited functionality or unsecure but very variable and I would vote for 2nd option.

personaly I prefer procedures in DB when trying to avoid injection and if parameter must be varchar I made it as short as possible with internal check in procedure or by regex in procedure caller, but this is totally different situation, because I know what this param should contain and what is invalid and must throw exception....

what do U think ?

 
New Post
9/13/2009 7:52 PM
 

The difference is that parameter values are passed separately to SQL Server and are sanitized by ADO.Net and SQL Server to escape SQL characters.  For example, the first ' character in the malicious string we've been using would be escaped, if we used parameters.  Token Replace, on the other hand, uses .Net String Replacement techniques.  Even if Token Replace is aware of SQL Injection concerns and performs the correctly sanitization, I'd much rather trust SQL Server to do it for me.

Using parameters, the query you wrote above would simply search for entries in mystore_Locations where the User = "'; UPDATE dnn_Users SET IsSuperUser = 1 WHERE UserId = 1234; SELECT '", the value in @Portal_user_name cannot find its way into the query.  So it's not a matter of what's more comfortable, its a matter of security.  Could the Reports Module implement Token Replace in a secure way by santizing the Token values?  Probably, but I would rather trust Microsoft to do it right than risk accidentally missing out some edge case.

On the topic of the trade off between security and functionality, I don't believe it is that much of a concern.  In this case, adding parameters would provide all the same features as Token Replace and be much more secure.  Trading security for functionality is asking for trouble and is not a trade off that the Reports Module will make under my leadership.  However, I think that when implemented correctly, almost all functionality can be implemented securely.


Andrew Nurse
DotNetNuke Core Team Member and Reports Module Project Lead
Microsoft Certified Professional Developer

 
Previous
 
Next
HomeHomeDNN Open Source...DNN Open Source...Module ForumsModule ForumsReportsReportsdnn Reports updated to VSE 2008 + token supportdnn Reports updated to VSE 2008 + token support


These Forums are dedicated to discussion of DNN Platform and Evoq Solutions.

For the benefit of the community and to protect the integrity of the ecosystem, please observe the following posting guidelines:

  1. No Advertising. This includes promotion of commercial and non-commercial products or services which are not directly related to DNN.
  2. No vendor trolling / poaching. If someone posts about a vendor issue, allow the vendor or other customers to respond. Any post that looks like trolling / poaching will be removed.
  3. Discussion or promotion of DNN Platform product releases under a different brand name are strictly prohibited.
  4. No Flaming or Trolling.
  5. No Profanity, Racism, or Prejudice.
  6. Site Moderators have the final word on approving / removing a thread or post or comment.
  7. English language posting only, please.