Best way to solve warning using dynamic OrderBy in AvanceSQL

Back to Forums

Carlos Olías

Solved

Question

SQL

Best Practices

Hi there!

I have the typical advanced SQL with the order by passed with ExpandInline = Yes, where OrderBy parameter is the sort column in table, form example: Id Desc

Now, I have a warning that says:

SQL Injection - When adding an expand inline parameter after a Group By or Order By, make sure to sanitize the parameter value. Otherwise, the action may be vulnerable to SQL injection.

In order to "sanitize the SQL parameters", do it in following best practices, and removing the IDE warning, what would be the best way?

If you use ENCODESQL, a new warning appear: "SQL Injection - EncodeSql should only be used to escape string literals. Check the EncodeSql documentation to learn how to use this function" ,so it doesn't seem to be the right choice
If try to use VerifySqlLiteral, warning disappear, but code fails, returning an exception form Sanitize function: "INVALID SQL LITERAL".
In other forums I have read, even more, that VerifySQLLiteral is deprecated, that should use "BuildSafe_InClauseTextList", but this generate an SQL like this:
SELECT {User}.* from {User} order by 'Id Desc'
and produce and SQL Error. I could cheat this, removing Quotes, doing something like this: SubStr(BuildSafe.out,1,Length(BuildSafe.out)-2), but it is like doing nothing, and it sure does not follow the best practices of Outsystems to avoid SQL Injection

So, how to do it following best practices and removing the warning from Service Studio?

Thanks,

18 Oct 2021

Carlos Olías

Solution

Hi there!

I am not forgetting this post, but it took me a long time to get an official response.

I have been talking with support team, and their answer is:

don't use expand inline (but it makes no sense, there is no alternative to when you want to do a subquery, or use a list of values)
hide the message if I follow the best practices that we have mention in this post.

But they have recognize that there is no "official" way to remove the warning (not just hide), and do it following best practices, and has send the problem to "development team".

See solution in context

15 Nov 2021

Márcio Carvalho

So can you check that on this post?

There is a really good explanation by @Dorine Boudry and Cristiana Umbelino

https://www.outsystems.com/forums/discussion/59321/sort-sql-query-in-reactive-web/#Post248415

Kind regards,

Márcio

18 Oct 2021

3 replies

Last reply 18 Oct 2021

Show thread

Hide thread

Carlos Olías

Hi Márcio,

I had checked this post before, and talk about some topics that I believe don't resolve entire question. Talk about Building Dynamic SQL Statements the Right Way, that redirect to this other topic, How to enable dynamic sorting in a table fed by a SQL query, that, In addition to making a function with a series of very manual replacements, it does not solve the warning that Outsystems gives (even if it solves the vulnerability itself). In one of the screenshot in the link you could se that warning still appears

Other people here also offer an oml, with the same solution, but, also, the warning is still present.

Is there anyway to, on the one hand, to solve the warning in a correct way, and on the other hand, that Outsystems detects that it has been solved and does not show the warning?

Thanks,

18 Oct 2021

Márcio Carvalho

Replying to Carlos Olías' comment on 18 Oct 2021 13:58:14

In this case, you can still follow the best practices but the warning will still be there.

The topics/articles you shared demonstrates a nice way of secure parameters with the expand inline!

Attention that the BuildSafe_In is when you want to use the IN clause among a list of data.

So from what I know now on this topic, and is not completely true what I am going to say, is that the warning is nothing to be afraid of, because is there for us to know that maybe it might have a security breach through the input parameter that has the expand inline. So maybe you can hide the warning.

On the other hand, I would still ask the support on Outsystems why that still happen nowadays.

ps: I would want to know how to clean that warning, and why we cannot because I am working and I have worked where that warning is still there....

18 Oct 2021

Carlos Olías

Replying to Márcio Carvalho's comment on 18 Oct 2021 14:38:39

I believe that the right answer is "there is no way to remove warning, and do it right". I'll ask Outsystems, to try to get an official asnwser.

18 Oct 2021

Vijay Malviya

Hi,

Once try the below code.

If(TableSort = "", "1", EncodeSql(TableSort)) + If (OrderIsAscending , " ASC", " DESC")

18 Oct 2021

1 reply

18 Oct 2021

Show thread

Hide thread

Carlos Olías

Hi Vijay,

I have checked your code, the problem is the same, Outsystems returns another warning

Regards

18 Oct 2021

Carlos Olías

Solution

Hi there!

I am not forgetting this post, but it took me a long time to get an official response.

I have been talking with support team, and their answer is:

don't use expand inline (but it makes no sense, there is no alternative to when you want to do a subquery, or use a list of values)
hide the message if I follow the best practices that we have mention in this post.

But they have recognize that there is no "official" way to remove the warning (not just hide), and do it following best practices, and has send the problem to "development team".

15 Nov 2021

Ross Jennings

OutSystems still haven't provided a good official solution to this problem (I don't know why they haven't built sanitization into the AdvancedSQL widget itself). However, there is now a comprehensive how-to guide for the recommendation approach, even though it still doesn't remove the Service Studio warning.

https://success.outsystems.com/documentation/how_to_guides/development/how_to_enable_dynamic_sorting_in_a_table_fed_by_a_sql_query/

24 May 2023

David Sousa

Hey Carlos,

If the problem is just with going around the warning you can to the following:

Call BuildSafe_InClauseTextList with an empty list (doesn't matter what you pass inside)
Assign your TableSort value to the BuildSafe_InClauseTextList.Output variable
Use BuildSafe_InClauseTextList.Output in the TableSort attribute in the SQL Query

By the way, I tested the solution of passing the TableSort inside BuildSafe_InClauseTextList and then applying the substring to remove both quotes and it works fine as well. Removes the warning and is secure against SQL injection since TableSort passed inside the sanitization action.

Let me know if it solved it for you.

17 Aug 2023

3 replies

Last reply 22 Apr 2024

Show thread

Hide thread

Mangini

Just stumbled uppon this today while trying to sort a similar warning.

Unfortunatelly this doesn't work anymore since platform version 11.23.1 (at least the ones that use SQL Server) due to a change Outsystems made in BuildSafe_InClauseTextList, it now adds a N' on its output, so stuff like N'SomeColumn DESC will throw an error when a Advanced SQL is run.