search

holyprin / Holyprin.Web.Security

ASP.NET Code-First Membership Provider with basic model abstraction
12 stars 3 forks source link

SQL Injection? #3

Closed Devristo closed 12 years ago

Devristo commented 12 years ago

I am not sure this is true, but it seems like the q() function does not escape the query parameters. In this case this might be okay (since usernames/roles are 99% of the times restricted to alphanumeric chars). But it might be better to escape them anyway (to prevent issues with future queries) or use prepared statements?

The q() method i am talking about is the one in the CFMembershipProvider and the CFRoleProvider.

holyprin commented 12 years ago

You are correct. that particular function would only allow me to use the single to double escaping technique which would still leave the base provider open in some way. I could maybe rewrite it to take sql parameters, or the final approach is to make absolutely certain to sanitize form inputs for stuff like this (should be a standard anyway). I'll look into building parametrization into the code though.

holyprin commented 12 years ago

turns out using parameters was simple enough, some decent lines of rewriting will probably tweak it in the future, but thanks for pointing it out. Code is updated.

Devristo commented 12 years ago

Awesome, thanks a lot! Your project makes building a website much easier. Since I can now easily add IUser navigation properties in my EF entities. Will update now.

holyprin commented 12 years ago

Not a problem, thanks for the good marks :-P If you notice anything else please bring it to my attention, this was a side project that has kinda dropped into something half decent, I wasn't 100% focused when I rewrote it for version 2 so there is bound to be some more bugs. But I will happily fix them if people post them. :-D