ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Le Roux (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OFBIZ-6567) Wrong percent encoding in Webtool/SQL Processor
Date Fri, 31 Jul 2015 11:48:04 GMT

    [ https://issues.apache.org/jira/browse/OFBIZ-6567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14649110#comment-14649110
] 

Jacques Le Roux commented on OFBIZ-6567:
----------------------------------------

That's an excellent idea Gareth, I see that you thought about it since we spoke about it :).
I was also reluctant to change UtilCodec.

Jacopo reused the [ESAPI DefaultEncoder canonicalize() method implementation|https://code.google.com/p/owasp-esapi-java/source/browse/trunk/src/main/java/org/owasp/esapi/reference/DefaultEncoder.java#sl_svn1961_139]
to decode requests parameters when he did the ESAPI refactoring to introduce ESAPI 2 because
of the possible double encoding or alike attacks. But as explained in this article http://security.coverity.com/blog/2013/Nov/to-escape-or-not-to-escape-that-is-the-question.html
canonicalizing paqrameters has possible "drawbacks" and you crossed one similar to the one
reported in this article.

We already knew there are issues with UtilCodec.canonicalize() as reported in OFBIZ-5910,
where the same article is referenced. And we also know there is no global solution. So at
least this aspect (LIKE in SQL processor) is now handled with your solution and I prefer to
keep the canonicalize method with percent and HTML entities decoding for now. As you suggested
in OFBIZ-5910 using Commons StringEscapeUtils class should be considered when crossing issues
with UtilCodec.canonicalize().

> Wrong percent encoding in Webtool/SQL Processor
> -----------------------------------------------
>
>                 Key: OFBIZ-6567
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-6567
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: Release Branch 12.04, Release Branch 13.07, Release Branch 14.12,
Trunk
>            Reporter: Jacques Le Roux
>            Priority: Minor
>
> This was reported to me by Gareth Carter, the investigation is mine.
> If for instance you use this SQL expression
> {code}
> select * from Party_Role where role_Type_Id LIKE  '%CA%'
> {code}
> It will be interpreted (and returned to UI) as
> {code}
> select * from Party_Role where role_Type_Id LIKE  'Ê%'
> {code}
> And no result will be returned when OOTB there is  6 <PartyRole partyId="***" roleTypeId="CARRIER"/>
entities
> This is because in UtilHttp.canonicalizeParameterMap() UtilHttp.canonicalizeParameter()
is called. And inside the later UtilCodec.canonicalize() is used. So 2 ESAPI codecs are tested
HTMLEntityCodec.decode() and PercentCodec.decode(). Only PercentCodec.decode() does a change
so it's picked. In this case it should not, because nothing should be decoded. At this point,
nothing has been encoded, the String the codec decodes is still "select * from Party_Role
where role_Type_Id LIKE  '%CA%'"
> I read at https://en.wikipedia.org/wiki/Percent-encoding that though mostly planned for
URL encoding  percent encoding
> bq. is also used in the preparation of data of the application/x-www-form-urlencoded
media type, as is often used in the submission of HTML form data in HTTP requests.
> But in the specific case of a like in an SQL expression coming from the text area of
webtools/control/EntitySQLProcessor it should not be used because the % followed by some chars,
may be wrongly decoded.
> Because there are no other ways provided by the percent codec to prevent the decoding
(it's supposed to have been encoded before), I'm not quite proud of it but I found only this
workaround so far
> {code}
> Index: framework/base/src/org/ofbiz/base/util/UtilCodec.java
> ===================================================================
> --- framework/base/src/org/ofbiz/base/util/UtilCodec.java	(revision 1693397)
> +++ framework/base/src/org/ofbiz/base/util/UtilCodec.java	(working copy)
> @@ -164,16 +164,24 @@
>              while (i.hasNext()) {
>                  Codec codec = i.next();
>                  String old = working;
> -                working = codec.decode(working);
> -                if (!old.equals(working)) {
> -                    if (codecFound != null && codecFound != codec) {
> -                        mixedCount++;
> +                String upperWorking = working.toUpperCase();
> +                if (codec instanceof PercentCodec
> +                        && upperWorking.contains("WHERE")
> +                        && upperWorking.contains("LIKE")
> +                        && upperWorking.contains("%")) {
> +                    continue;
> +                } else {
> +                    working = codec.decode(working);
> +                    if (!old.equals(working)) {
> +                        if (codecFound != null && codecFound != codec) {
> +                            mixedCount++;
> +                        }
> +                        codecFound = codec;
> +                        if (clean) {
> +                            foundCount++;
> +                        }
> +                        clean = false;
>                      }
> -                    codecFound = codec;
> -                    if (clean) {
> -                        foundCount++;
> -                    }
> -                    clean = false;
>                  }
>              }
>          }
> {code}
> Better ideas?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message