How to resolve SQL Injection Warning

I am getting following SQL Injection warning in my application.

Ensure the expand inline argument is protected by using EncodeSql(), or VerifySqlLiteral() from the Sanitization extension, to avoid security flaws.

I have parameter called @Ids with Expand Inline enabled. @Ids parameter will contain Employee Ids with comma separated. Eg: "1,2"

Query: SELECT * FROM {Employee} WHERE {Employee}.[Id] in (@Ids)

If I disable the Expand Inline property then I am getting SQL error.

Error converting data type nvarchar to bigint.

Please advice how to resolve this warning.


Hi Bala,

Did you checked what exactly is going in your @Ids parameter , it should be like (1,2,3) . Sometimes when you create it in a loop you end up with an additional in the end like (1,2,3,) .

Just try to print the Query in Audit log and see how the entire query is being created.


Regards

-PJ-


Hi Balamurugan,

I'm not getting why its happening so (it's not showing error in my case) but could you please pass the Advance Query input argument for Ids as: EncodeSql(Ids)

Query: SELECT {Employee}.* FROM {Employee} WHERE {Employee}.[Id] in (@Ids)


Kind Regards,

Benjith Sam

Yes Benjith,

You are correct the EncodeSql function is to prevent Sql injection but the error is i think while running the query , it s not the warning.


Regards

-PJ-

PRAMOD JAIN wrote:

Yes Benjith,

You are correct the EncodeSql function is to prevent Sql injection but the error is i think while running the query , it s not the warning.


Regards

-PJ-

Hi Sir,

Bala mentioned in his post that he is trying to resolve the warning message.

Balamurugan wrote:

Please advice how to resolve this warning.



Kind Regards,

Benjith Sam

Solution

Hi,


First of all, sorry that it's hard to understand how to solve these warnings. We at OutSystems are working in improving them and updating both warnings and the documentation in the following weeks to address most of the confusion.


In your case, if before your query are doing an iteration on a list of integers or integer identifiers to concatenate a comma separated string, then the warning is a false positive. Since there is no chance for a end user to tamper it in a way that introduces a sql injection problem.

So for this particular case the best is to hide the warning.


Just to give bit more context about the EncodeSQL, since it was mentioned: doing EncodeSQL(ids) is wrong. It will make the warning disappear but it is not really protecting anything. We will add a new explicit warning for this scenario.

A correct usage for it would be if the ids were of type TextIdentifier. In that case the concatenations would have to look like:

csvIds = If(csvIds <>"", ",","") + "'" + EncodeSQL(List.Current.SomeTextId) +"'"

This would make your csvIds variable safe from sql injection problems but (on the current version) you would still need to hide the warning.


Regards,

João Rosado

Solution