EncodeSQL and OrderBy Expand Inline

EncodeSQL and OrderBy Expand Inline

  

Hi,

I'm surprised I couldn't find any post related with this. But here's the thing:

At my company we've been discussing how and when to use EncodeSQL, and if we should or not use it for the OrderBy variable which is usually an inline parameter.

We have agreed to not encode the OrderBy variable, but according to this link, it kind of looks like we should? Or maybe it's just a miss understanding of the document...

So, by not using it, we'll end up with a warning in Development Environment, so if we shouldn't use it, maybe the Development Environment should have a different message, or a different type of message, don't know.

Anyway, what are your opinions? And if someone from OutSystems R&D sees this, hey, let us know your thoughts.

Cheers,

Nelson

Hi,

If your OrderBy variable can be directly changed by application Users, then you must be careful when using it as an inline parameter in your Advanced Queries, as you'll enable SQL injection.

If you are sure it does not present a security risk, as in you are in absolute control of possible values, and you don't mind having the warning, then there should be no problem not encoding the variable.

Note: If your Outsystems applications will be subject to Outsystems Code Review you should not have any warnings, however, in that case, you shouldn't be using Inline parameters anyway, as they prevent the database from optimizing execution plans.

Cheers,

João

Hi Nelson,

I think the description is chosen a bit poorly, especially in combination with the example. You should use EncodeSql if, and only if, you are going to use the Expand Inline Parameter as an SQL string literal. It's purpose is to prevent SQL injection, but with e.g. an Order By you are _actually trying to inject SQL_ in the query, so using an EncodeSql could screw up your "injected" SQL.

EncodeSql is useful if the source of the literal you are trying to expand inline is user input, database, REST service etc., viz. all external sources. An Order By (or, something we use extensively, a (part of a) Where clause) or similar that's constructed in the code has no need for it, and would actually screw up string literals inside the injected SQL.

As for the warnings, you can just hide them where appropriate.

Hi João and Kilian,

thank you for your feedback.

I am sure that our "Orderbys" are never inputed by our users, so regarding SQL injection for that specific case, we are safe.

I personally don't like to hide warnings because you might have a hidden warning that will actually get random runtime errors, but since it was hidden, you didn't know anything about it. This is more for juniors who hide stuff just because....

I prefer to have it visible and see by its description that it might be related with Order by expand inlines.

Hi Nelson,

I think it's a trade-off; when you hide the proper warnings (and only those you are sure can be ignored!), everything else that pops up gets immidiate attention. On the other hand, you're not seeing warnings that may need attending to in some future timeframe. But in general, the Platform is sometimes a bit too trigger-happy when it comes to warnings (like the ones for For Each over a Table Records' list, not sure whether it's stil in there) imho, and you can safely hide those that you're sure are covered.