2023-09-13 13-47-19
Nuno Magalhães
890Views
15Comments
Solved
Sort SQL Query in Reactive Web
Question
Reactive
OutSystems Widgets

How can we do a table sort in a SQL Query in Reactive Web so that we don't have the warning of SQL Injection?

I know that the EncodeSQL is useless here and BuildSafe_InClauseList from Sanitization is also useless. Reactive automatically creates a TableSort variable and an action OnSort on the respective table but if we pass this client value as an Expand Inline it will give the correct warning of SQL Injection. This is specially dangerous since we're dealing with client side code where variables can be easily tampered.

Help, please.

0
0
03 Apr 2020
Solution

Hi All,

As Nuno already said, those sanitation type of actions work on sql literals, not on parts of the sql clause, as sort by is.

So, as Vincent is saying, I don't think you can necessarily make the warning go away.  But you can still mitigate the dangers this warning is for, right ?

I don't know how complex your sorting is, but if it's just the standard "click on the column header to sort by that attribute, click again to sort descending" type of approach, then just build a function checking the TableSort string against the attributes in the entity, anything else, log a message and apply some standard sorting.

See attached oml for a simple reactive example, it will of course get more complex as soon as the table has data that are result of a join, or if sorting on several columns is combined, but that would just be some more complex logic in the function that turns a TableSort string into a safe OrderBy clause.

Have a look at the oml and let me know if this sort of thing is what you are looking for.

Dorine

See solution in context
DemoTableSortInSqlNoInjection.oml
1
0
05 Aug 2020

Dorine Boudry wrote:

Hi All,

As Nuno already said, those sanitation type of actions work on sql literals, not on parts of the sql clause, as sort by is.

So, as Vincent is saying, I don't think you can necessarily make the warning go away.  But you can still mitigate the dangers this warning is for, right ?

I don't know how complex your sorting is, but if it's just the standard "click on the column header to sort by that attribute, click again to sort descending" type of approach, then just build a function checking the TableSort string against the attributes in the entity, anything else, log a message and apply some standard sorting.

See attached oml for a simple reactive example, it will of course get more complex as soon as the table has data that are result of a join, or if sorting on several columns is combined, but that would just be some more complex logic in the function that turns a TableSort string into a safe OrderBy clause.

Have a look at the oml and let me know if this sort of thing is what you are looking for.

Dorine

 

Just to add some color to this reply.
The main concern is security and that's why that warning appears. As long as you can come up with a solution that you are confident it's reliable the warning is no longer a concern. Another thing to have in mind is that we probably will need to have a trade-off here. My two cents on this is that the most reasonable one is to have a bit more code than usual as long as it is easily understandable to ease on maintenance and troubleshooting. I tend to avoid dynamic SQL clauses because of that.


From what I could test, this was the best I could achieve:

Have the attributes listed mapped with an alias that will not be showed in runtime. The "Name" attribute could map with the "SortByName" alias and the OrderBy clause will only work for the attributes that are mapped, always falling back to a default one chosen at design time. The trade-off is that whenever there's a need to change attributes this code will also need to be changed. Also, the warning will still appear but you are now certain that your code is secure.

2
0
05 Aug 2020
Solution

The following documents describe your situation but it doesn't mention how to create an injectable without the warning. I don't think it possible, I never could find a solution for this warning.

https://success.outsystems.com/Documentation/11/Reference/Errors_and_Warnings/Warnings/SQL_Injection_Warning

0
0
03 Apr 2020

Vincent Koning wrote:

The following documents describe your situation but it doesn't mention how to create an injectable without the warning. I don't think it possible, I never could find a solution for this warning.

https://success.outsystems.com/Documentation/11/Reference/Errors_and_Warnings/Warnings/SQL_Injection_Warning


Thanks, but I already went through the Outsystems documentation in this regard. That's why I'm lost if this is possible or not.

0
0
03 Apr 2020

Any updates on this topic? 

0
0
05 Aug 2020

Hi,

you're talking about advance queries, right?


Did you try this:

Cheers

0
0
05 Aug 2020

Miguel Verdasca wrote:

Hi,

you're talking about advance queries, right?


Did you try this:

Cheers

"Deprecated Action

The VerifySqlLiteral action is being deprecated. Check the BuildSafe_InClauseIntegerList and BuildSafe_InClauseTextList actions instead." :)

 

0
0
05 Aug 2020
Solution

Hi All,

As Nuno already said, those sanitation type of actions work on sql literals, not on parts of the sql clause, as sort by is.

So, as Vincent is saying, I don't think you can necessarily make the warning go away.  But you can still mitigate the dangers this warning is for, right ?

I don't know how complex your sorting is, but if it's just the standard "click on the column header to sort by that attribute, click again to sort descending" type of approach, then just build a function checking the TableSort string against the attributes in the entity, anything else, log a message and apply some standard sorting.

See attached oml for a simple reactive example, it will of course get more complex as soon as the table has data that are result of a join, or if sorting on several columns is combined, but that would just be some more complex logic in the function that turns a TableSort string into a safe OrderBy clause.

Have a look at the oml and let me know if this sort of thing is what you are looking for.

Dorine

DemoTableSortInSqlNoInjection.oml
1
0
05 Aug 2020

Dorine Boudry wrote:

Hi All,

As Nuno already said, those sanitation type of actions work on sql literals, not on parts of the sql clause, as sort by is.

So, as Vincent is saying, I don't think you can necessarily make the warning go away.  But you can still mitigate the dangers this warning is for, right ?

I don't know how complex your sorting is, but if it's just the standard "click on the column header to sort by that attribute, click again to sort descending" type of approach, then just build a function checking the TableSort string against the attributes in the entity, anything else, log a message and apply some standard sorting.

See attached oml for a simple reactive example, it will of course get more complex as soon as the table has data that are result of a join, or if sorting on several columns is combined, but that would just be some more complex logic in the function that turns a TableSort string into a safe OrderBy clause.

Have a look at the oml and let me know if this sort of thing is what you are looking for.

Dorine

 

Just to add some color to this reply.
The main concern is security and that's why that warning appears. As long as you can come up with a solution that you are confident it's reliable the warning is no longer a concern. Another thing to have in mind is that we probably will need to have a trade-off here. My two cents on this is that the most reasonable one is to have a bit more code than usual as long as it is easily understandable to ease on maintenance and troubleshooting. I tend to avoid dynamic SQL clauses because of that.


From what I could test, this was the best I could achieve:

Have the attributes listed mapped with an alias that will not be showed in runtime. The "Name" attribute could map with the "SortByName" alias and the OrderBy clause will only work for the attributes that are mapped, always falling back to a default one chosen at design time. The trade-off is that whenever there's a need to change attributes this code will also need to be changed. Also, the warning will still appear but you are now certain that your code is secure.

2
0
05 Aug 2020

Hi,

just to add, you can check this information:

Building Dynamic SQL Statements the Right Way

Cheers 

0
0
05 Aug 2020
2022-05-19 09-39-13
David Sousa
Champion

Hey everyone,

It seems that the IDE is pleased when something that comes out from BuildSafe_InClauseTextList is passed to an expand inline parameter, so here are 2 solutions that are safe and have no warnings.


Solution 1)

  • Call BuildSafe_InClauseTextList (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



Solution 2)

  • Passing the TableSort inside BuildSafe_InClauseTextList


  • Apply a substring to remove both quotes


  • Pass the resulting value to the TableSort attribute SQL Query


Cheers,

David

0
0
17 Aug

Hi David,

I don't think either of these options are in themselves 'safe' against sql injection.  They are tricks to make the warning go away, but if you just take the TableSort variable form client side, it can be tampered with, so risk of sql injection is still there without any warning.

Also, the buildsafeInClause is not meant to build a sort clause, which creates very confusing code for later developers to maintain.

Dorine

0
0
17 Aug
2022-05-19 09-39-13
David Sousa
Champion

Replying to Dorine Boudry's comment on 17 Aug 2023 16:32:31

Hey Dorine,

I was just answering to the original question about getting rid of the warnings :)

Regarding security against injection, in the second solution I shared the TableSort value goes through BuildSafe_InClauseTextList, so safety is ensured. The only thing performed afterwards is clearing the quotes, so that the value can be used in the sort condition of the query.

I know it's not an obvious solution but if one wants to clear the warning and keep the code safe against injection I'd say the second solution works well until we have a proper way of coding it.

Cheers!

0
0
22 Aug

It really isn't in any way safe.  

All that BuildSafe action is doing, is adding single quotes (and comma's if there were more items), and you subsequently remove them.

So

  • a hacker can just put something like Name DESC; delete from {user} into the TableSort variable
  • the buildsafe will turn that into 'Name DESC; delete from {user} '
  • your assign will turn it back into  Name DESC; delete from {user} 
  • and BOOM : all users are gone 

Dorine

0
0
22 Aug

Replying to You's comment on 22 Aug 2023 16:29:19

Oh, and for "proper way of coding it", safety is only going to come from validating on server side if the variable contains a valid attribute of the desired entity

0
0
22 Aug
2022-05-19 09-39-13
David Sousa
Champion

Replying to Dorine Boudry's comment on 22 Aug 2023 16:29:19

Thanks for the warning about security. Indeed the BuildSafe action isn't doing any safety validations against injection. Thought it would be more secure since it's the recommended action to use in the deprecation warning of VerifySQLLiteral. 

I've checked this solution against injection and it seems to work well.

So that + any of the solutions shared above would work against injection and have no warnings in Service Studio.

1
0
23 Aug
Community GuidelinesBe kind and respectful, give credit to the original source of content, and search for duplicates before posting.