You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by "Ing. Andrea Vettori" <a....@b2bires.com> on 2020/01/15 10:24:13 UTC
Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Hello,
Today I had an issue with one of our web sites using struts and found it traces back to the default accepted patterns in DefaultAcceptedPatternsChecker.
May I ask why the key values for “map like” parameters (i.e. map[‘key’]) are limited in such a strict way ? In our case I had a minus sign in the key. Is there any security consideration behind this ?
Thank you!
public static final String[] ACCEPTED_PATTERNS = {
"\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
};
—
Ing. Andrea Vettori
Responsabile Sistemi Informativi
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by John Bush <jt...@mchsi.com>.
Yes I've had to use misc concats, weird escaping, even making sure the
string portion of the name was first to make sure it didn't treat it
mathematically and to keep the taglibs from interfering with my names.
Mixing front end frameworks leads to weird results sometimes also since
they resolve the <tag>contents</tag> to what makes sense for them. Over
the years I just got accustomed to avoiding 'specials' in names in
languages not strongly typed.
John !:)
On 1/29/2020 5:48 AM, Ing. Andrea Vettori wrote:
>> On 29 Jan 2020, at 01:48, John Bush <jt...@mchsi.com> wrote:
>>
>> I've had problems in some frameworks confusing the parser using operators this way. Since java maps allow any object as the key, such as Number or it's sub-classes, you get into issues where the page tech (jsp,js,various taglibs,et al) makes Map[key-1] into Map[1-1] then into Map[0]. I basically stay away from arithmetic operators unless I am performing math for that reason. I think what is there is reasonable for the wider audience and that providing the ability to specify your own is a reasonable solution.
>
> Does that happen even when you use the ’string’ format i.e. you make it explicit that it’s a string using the quotes ?
>
> —
> Ing. Andrea Vettori
> Responsabile Sistemi Informativi
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by "Ing. Andrea Vettori" <a....@b2bires.com>.
> On 29 Jan 2020, at 01:48, John Bush <jt...@mchsi.com> wrote:
>
> I've had problems in some frameworks confusing the parser using operators this way. Since java maps allow any object as the key, such as Number or it's sub-classes, you get into issues where the page tech (jsp,js,various taglibs,et al) makes Map[key-1] into Map[1-1] then into Map[0]. I basically stay away from arithmetic operators unless I am performing math for that reason. I think what is there is reasonable for the wider audience and that providing the ability to specify your own is a reasonable solution.
Does that happen even when you use the ’string’ format i.e. you make it explicit that it’s a string using the quotes ?
—
Ing. Andrea Vettori
Responsabile Sistemi Informativi
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by John Bush <jt...@mchsi.com>.
I've had problems in some frameworks confusing the parser using
operators this way. Since java maps allow any object as the key, such as
Number or it's sub-classes, you get into issues where the page tech
(jsp,js,various taglibs,et al) makes Map[key-1] into Map[1-1] then into
Map[0]. I basically stay away from arithmetic operators unless I am
performing math for that reason. I think what is there is reasonable for
the wider audience and that providing the ability to specify your own is
a reasonable solution.
John (imho)
On 1/28/2020 6:17 AM, Lukasz Lenart wrote:
> pon., 27 sty 2020 o 16:59 Ing. Andrea Vettori<a....@b2bires.com>
> napisał(a):
>>> On 26 Jan 2020, at 12:26, Lukasz Lenart<lu...@apache.org> wrote:
>>>
>>>
>>> So maybe relaxing the patterns is a good idea but as till now nobody
>>> reported any problems with them, we decided to left them as is.
>> Thank you for the clarification.
>> If you’re going to discuss again this choice in the future, I think that allowing all characters that are safe on http post data or urls would be nice (that was the reason we choose to use the minus instead of other characters in our project, because it would work in post and urls).
> Feel free to open a ticket with your example pattern and we can work
> on that in Struts 2.6
>
>
> Regards
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by Lukasz Lenart <lu...@apache.org>.
pon., 27 sty 2020 o 16:59 Ing. Andrea Vettori <a....@b2bires.com>
napisał(a):
>
> > On 26 Jan 2020, at 12:26, Lukasz Lenart <lu...@apache.org> wrote:
> >
> >
> > So maybe relaxing the patterns is a good idea but as till now nobody
> > reported any problems with them, we decided to left them as is.
>
>
> Thank you for the clarification.
> If you’re going to discuss again this choice in the future, I think that allowing all characters that are safe on http post data or urls would be nice (that was the reason we choose to use the minus instead of other characters in our project, because it would work in post and urls).
Feel free to open a ticket with your example pattern and we can work
on that in Struts 2.6
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by "Ing. Andrea Vettori" <a....@b2bires.com>.
> On 26 Jan 2020, at 12:26, Lukasz Lenart <lu...@apache.org> wrote:
>
>
> So maybe relaxing the patterns is a good idea but as till now nobody
> reported any problems with them, we decided to left them as is.
Thank you for the clarification.
If you’re going to discuss again this choice in the future, I think that allowing all characters that are safe on http post data or urls would be nice (that was the reason we choose to use the minus instead of other characters in our project, because it would work in post and urls).
Thanks for the great work with Struts.
—
Ing. Andrea Vettori
Responsabile Sistemi Informativi
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by Lukasz Lenart <lu...@apache.org>.
niedz., 26 sty 2020 o 12:08 Ing. Andrea Vettori
<a....@b2bires.com> napisał(a):
>
> Thanks for you answer. I’ll try to look into the struts sources but I’m not sure to have understand your answer.
> What I’m trying to understand is why when we use an input like
>
> <input type="text" name=“map[‘key']” …..>
>
> and we use an action with a
>
> Map<String,String> map
>
> property, the key must match (\\w|[\\u4e00-\\u9fa5])+
>
> I tried to add this to my struts project
>
> struts.additional.acceptedPatterns=\\w+\\['(\\w|-)+'\\]
>
> and now keys with the minus symbol are working as expected and haven’t noticed any other issues.
>
> I also tried the following code and it works as expected (i.e. it prints {key-1=b, key=a}).
> I have NOT looked into struts source (I’ll try) so excuse me if the example is not relevant.
>
>
> public void doWork() throws Exception {
>
> Bean bean = new Bean();
> Ognl.getValue("map['key']='a'", bean);
> Ognl.getValue("map['key-1']='b'", bean);
>
> System.out.println(bean.getMap());
>
> }
>
> class Bean {
> private Map<String,String> map;
>
> public Bean() {
> map = new HashMap<>();
> }
>
> public Map<String,String> getMap() {
> return map;
> }
>
> public void setMap(Map<String,String> map) {
> this.map = map;
> }
> }
Maybe I will try to clarify this. "struts.additional.acceptedPatterns"
was the very first idea before we introduced some other mechanisms in
our Struts <-> OGNL integration bridge. One of those mechanisms is to
avoid nested/eval/chained expressions [1], which basically blocks
evaluating an expression-in-expression e.g.
"${myValue[$otherValue-1]}" as such expression can be dangerous as
they are run inside OGNL playground, out of Struts control.
So maybe relaxing the patterns is a good idea but as till now nobody
reported any problems with them, we decided to left them as is.
[1] https://github.com/apache/struts/blob/master/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java#L445-L456
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by "Ing. Andrea Vettori" <a....@b2bires.com>.
Thanks for you answer. I’ll try to look into the struts sources but I’m not sure to have understand your answer.
What I’m trying to understand is why when we use an input like
<input type="text" name=“map[‘key']” …..>
and we use an action with a
Map<String,String> map
property, the key must match (\\w|[\\u4e00-\\u9fa5])+
I tried to add this to my struts project
struts.additional.acceptedPatterns=\\w+\\['(\\w|-)+'\\]
and now keys with the minus symbol are working as expected and haven’t noticed any other issues.
I also tried the following code and it works as expected (i.e. it prints {key-1=b, key=a}).
I have NOT looked into struts source (I’ll try) so excuse me if the example is not relevant.
public void doWork() throws Exception {
Bean bean = new Bean();
Ognl.getValue("map['key']='a'", bean);
Ognl.getValue("map['key-1']='b'", bean);
System.out.println(bean.getMap());
}
class Bean {
private Map<String,String> map;
public Bean() {
map = new HashMap<>();
}
public Map<String,String> getMap() {
return map;
}
public void setMap(Map<String,String> map) {
this.map = map;
}
}
Thank you
—
Ing. Andrea Vettori
Responsabile Sistemi Informativi
> On 25 Jan 2020, at 11:40, Yasser Zamani <ya...@apache.org> wrote:
>
> Hi,
>
> AFAIK Ognl compiles myMap['myKey'] to the string myMap.myKey so yes I think you don't want to use complex strings as keys; for instance conceive `myKey-1` as key that will be translated to myMap.myKey-1 which likely won't work properly.
>
> Regards.
>
>> -----Original Message-----
>> From: Ing. Andrea Vettori <a....@b2bires.com>
>> Sent: Wednesday, January 15, 2020 1:54 PM
>> To: dev@struts.apache.org
>> Subject: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
>>
>> Hello,
>> Today I had an issue with one of our web sites using struts and found it traces
>> back to the default accepted patterns in DefaultAcceptedPatternsChecker.
>> May I ask why the key values for “map like” parameters (i.e. map[‘key’]) are
>> limited in such a strict way ? In our case I had a minus sign in the key. Is there any
>> security consideration behind this ?
>> Thank you!
>>
>>
>> public static final String[] ACCEPTED_PATTERNS = {
>> "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-
>> \\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
>> };
>>
>> —
>> Ing. Andrea Vettori
>> Responsabile Sistemi Informativi
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
RE: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
Posted by Yasser Zamani <ya...@apache.org>.
Hi,
AFAIK Ognl compiles myMap['myKey'] to the string myMap.myKey so yes I think you don't want to use complex strings as keys; for instance conceive `myKey-1` as key that will be translated to myMap.myKey-1 which likely won't work properly.
Regards.
>-----Original Message-----
>From: Ing. Andrea Vettori <a....@b2bires.com>
>Sent: Wednesday, January 15, 2020 1:54 PM
>To: dev@struts.apache.org
>Subject: Standard Accepted Patterns in DefaultAcceptedPatternsChecker
>
>Hello,
>Today I had an issue with one of our web sites using struts and found it traces
>back to the default accepted patterns in DefaultAcceptedPatternsChecker.
>May I ask why the key values for “map like” parameters (i.e. map[‘key’]) are
>limited in such a strict way ? In our case I had a minus sign in the key. Is there any
>security consideration behind this ?
>Thank you!
>
>
> public static final String[] ACCEPTED_PATTERNS = {
> "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-
>\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
> };
>
>—
>Ing. Andrea Vettori
>Responsabile Sistemi Informativi