jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

Auto interface SQL injection? #460

Closed zhangb99 closed 3 years ago

zhangb99 commented 3 years ago

We have an auto interface like this

    [Sql("GetSomething")]
    public Task<Something> GetSomething(string id);

    [Sql("SELECT something FROM somewhere WHERE id=@id")]
    public Task<Something> SelectSomething(string id);

Whether SQL attribute contains a proc, or a parameterized text, I would expect it being safe against SQL injection. But we were surprised, it actually trying to execute additional SQL. Say id=abc returns some data

If you pass id=abc';IF%20(1=1)%20WAITFOR%20DELAY%20'0:0:5';-- it waits for 5 seconds If you pass id=abc';select%20cast(user%20as%20int);-- it shows current SQL service account name.

I tried all combinations, explicit parameter, specify CommandType, Proc vs Text, it appears auto interface SQL Attribute always tries to construct dynamic SQL, which result in SQL injection.

Did I miss something? Or using this the wrong way?

Thanks.

jonwagner commented 3 years ago

That’s surprising. It should use parameters for everything, except for one case where you’re passing in a list of id fields.

I’m working on an updated build for another open bug so I will investigate this as well.

On Jun 23, 2021, at 1:10 PM, zhangb99 @.***> wrote:



We have an auto interface like this

[Sql("GetSomething")]
public Task<Something> GetSomething(string id);

[Sql("SELECT something FROM somewhere WHERE ***@***.***")]
public Task<Something> SelectSomething(string id);

Whether SQL attribute contains a proc, or a parameterized text, I would expect it being safe against SQL injection. But we were surprised, it actually trying to execute additional SQL. Say id=abc returns some data

If you pass id=xyz';IF%20(1=1)%20WAITFOR%20DELAY%20'0:0:5';-- it waits for 5 seconds If you pass id=xyz';select%20cast(user%20as%20int);-- it shows current SQL service account name.

I tried all combinations, explicit parameter, specify CommandType, Proc vs Text, it appears auto interface SQL Attribute always tries to construct dynamic SQL, which result in SQL injection.

Did I miss something? Or using this the wrong way?

Thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/460, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMTO5BZPMFDFMCTM6EJTUTTUIPTBANCNFSM47GKA6EA.

zhangb99 commented 3 years ago

Sorry, my bad, I had a dynamic SQL within the proc.