You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2009/04/21 18:25:40 UTC

Securing URLs in Freemarker templates

I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which were ending as a mess and opened a Jira task with already 3
sub-tasks (taken from OFBIZ-2260)

There are also some exceptions like we found in the widget part (strings with dynamic params names and value), see for instance
OFBIZ-2332.

So at this stage we are caught, we don't accept such URLs but there are hard to change. And unfortunately this scheme is pretty
often used
6 ${paramString}</...@ofbizUrl>"
26 ${paramList}</...@ofbizUrl>"
4 ${parameters.targetRequestUri}</...@ofbizUrl>"
I'm not sure there are no other cases.
I believe we should think about a solution for all these exceptions... Maybe rewriting upstream as David already suggested.

Also, in order to get more feedbacks, I'd like to add
    "Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 (check before
if a sub-task for this error does not exist). If you are not sure how to create a Jira issue please have a look before at
http://docs.ofbiz.org/x/r. Thank you in advance for your help"
at the end of the current error message which is something like
    Found URL parameter [partyId] passed to secure (https) request-map with uri [searchorders] with an event that calls service
[findOrders]; this is not allowed for security reasons! The data should be encrypted by making it part of the request body (a form
field) instead of the request URL.; In session [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be changed using the
service.http.parameters.require.encrypted property in the url.properties file

What do you think ?

Jacques



Re: Securing URLs in Freemarker templates

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Ashish,

For the moment I did not find enough time to look seriously at this. So only manual changes are use for now. I hope to have a look 
at this next week

Jacques

From: "Ashish Nagar" <as...@hotwaxmedia.com>
> Thanks Jacques,
>
> https://issues.apache.org/jira/browse/OFBIZ-2260 was really getting
> messed up with every new reported issue. Idea of creating new subtasks
> for each reported bug is good to persist. But I am concerned about,
> there may be a lot occurrences of these issues, so fixing each
> individual of these by hand (immediate solution) will be a hard work.
> Could there be some way to automate this task?
>
> Regards,
> --
> Ashish Nagar
>
> Jacques Le Roux wrote:
>> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which
>> were ending as a mess and opened a Jira task with already 3
>> sub-tasks (taken from OFBIZ-2260)
>>
>> There are also some exceptions like we found in the widget part
>> (strings with dynamic params names and value), see for instance
>> OFBIZ-2332.
>>
>> So at this stage we are caught, we don't accept such URLs but there
>> are hard to change. And unfortunately this scheme is pretty
>> often used
>> 6 ${paramString}</...@ofbizUrl>"
>> 26 ${paramList}</...@ofbizUrl>"
>> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
>> I'm not sure there are no other cases.
>> I believe we should think about a solution for all these exceptions...
>> Maybe rewriting upstream as David already suggested.
>>
>> Also, in order to get more feedbacks, I'd like to add
>>    "Moreover it would be kind if you could create a Jira sub-task of
>> https://issues.apache.org/jira/browse/OFBIZ-2330 (check before
>> if a sub-task for this error does not exist). If you are not sure how
>> to create a Jira issue please have a look before at
>> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
>> at the end of the current error message which is something like
>>    Found URL parameter [partyId] passed to secure (https) request-map
>> with uri [searchorders] with an event that calls service
>> [findOrders]; this is not allowed for security reasons! The data
>> should be encrypted by making it part of the request body (a form
>> field) instead of the request URL.; In session
>> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be changed
>> using the
>> service.http.parameters.require.encrypted property in the
>> url.properties file
>>
>> What do you think ?
>>
>> Jacques
>>
>>
> 



Re: Securing URLs in Freemarker templates

Posted by Ashish Nagar <as...@hotwaxmedia.com>.
Thanks Jacques,

https://issues.apache.org/jira/browse/OFBIZ-2260 was really getting 
messed up with every new reported issue. Idea of creating new subtasks 
for each reported bug is good to persist. But I am concerned about, 
there may be a lot occurrences of these issues, so fixing each 
individual of these by hand (immediate solution) will be a hard work. 
Could there be some way to automate this task?

Regards,
--
Ashish Nagar

Jacques Le Roux wrote:
> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which 
> were ending as a mess and opened a Jira task with already 3
> sub-tasks (taken from OFBIZ-2260)
>
> There are also some exceptions like we found in the widget part 
> (strings with dynamic params names and value), see for instance
> OFBIZ-2332.
>
> So at this stage we are caught, we don't accept such URLs but there 
> are hard to change. And unfortunately this scheme is pretty
> often used
> 6 ${paramString}</...@ofbizUrl>"
> 26 ${paramList}</...@ofbizUrl>"
> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
> I'm not sure there are no other cases.
> I believe we should think about a solution for all these exceptions... 
> Maybe rewriting upstream as David already suggested.
>
> Also, in order to get more feedbacks, I'd like to add
>    "Moreover it would be kind if you could create a Jira sub-task of 
> https://issues.apache.org/jira/browse/OFBIZ-2330 (check before
> if a sub-task for this error does not exist). If you are not sure how 
> to create a Jira issue please have a look before at
> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
> at the end of the current error message which is something like
>    Found URL parameter [partyId] passed to secure (https) request-map 
> with uri [searchorders] with an event that calls service
> [findOrders]; this is not allowed for security reasons! The data 
> should be encrypted by making it part of the request body (a form
> field) instead of the request URL.; In session 
> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be changed 
> using the
> service.http.parameters.require.encrypted property in the 
> url.properties file
>
> What do you think ?
>
> Jacques
>
>


Re: Securing URLs in Freemarker templates

Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks David,

I knew that :/, So I commited a change at r767694  in trunk and r767695 in R9.04

Hope there are no drawbacks, I can't see any

Jacques

From: "David E Jones" <da...@hotwaxmedia.com>
> 
> Here is an example of one place in an FTL file that does this (and  
> there are a few other examples around):
> 
> ${StringUtil.wrapString(productPromo.promoText?if_exists)}
> 
> The basic rule is very simple: if it is a java.lang.String object then  
> it will be HTML/XML encoded in FTL templates! If you don't want the  
> encode, then make it not a String.
> 
> -David
> 
> 
> On Apr 22, 2009, at 4:06 PM, Jacques Le Roux wrote:
> 
>> I have added more information to the error message at r767688 for  
>> the trunk and r767689 for R9.04.
>>
>> BTW, we now have <br> instead of real new line in error messages, is  
>> there not a quick fix for that ?
>>
>> Jacques
>>
>> From: "Jacques Le Roux" <ja...@les7arts.com>
>>> From: "David E Jones" <da...@hotwaxmedia.com>
>>>> This practice is a bad one in general, and is usually used to   
>>>> implement something like going back to a common page that is  
>>>> better  done with newer framework features like the last-view  
>>>> stuff in the  controller.xml file.
>>>> What's worse is that using these sorts of things often introduces   
>>>> security vulnerabilities because it is VERY easy to spoof  
>>>> parameters  and then they are treated like internal data (and  
>>>> that's aside from  the open URL versus encrypted form field issue).
>>>> So yes, it would be best to get rid of all of these. Unfortunately  
>>>> it  will take some work... on the other hand it looks like a lot  
>>>> of the  code that uses those things is pretty messy and it will be  
>>>> a huge  value add to clean them up (ie make the code easier to  
>>>> maintain,  customize, reuse, etc).
>>> And what about enhancing the error message ?
>>> Jacques
>>>> -David
>>>> On Apr 21, 2009, at 10:25 AM, Jacques Le Roux wrote:
>>>>> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260  
>>>>> which  were ending as a mess and opened a Jira task with already 3
>>>>> sub-tasks (taken from OFBIZ-2260)
>>>>>
>>>>> There are also some exceptions like we found in the widget part   
>>>>> (strings with dynamic params names and value), see for instance
>>>>> OFBIZ-2332.
>>>>>
>>>>> So at this stage we are caught, we don't accept such URLs but  
>>>>> there  are hard to change. And unfortunately this scheme is pretty
>>>>> often used
>>>>> 6 ${paramString}</...@ofbizUrl>"
>>>>> 26 ${paramList}</...@ofbizUrl>"
>>>>> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
>>>>> I'm not sure there are no other cases.
>>>>> I believe we should think about a solution for all these   
>>>>> exceptions... Maybe rewriting upstream as David already suggested.
>>>>>
>>>>> Also, in order to get more feedbacks, I'd like to add
>>>>>  "Moreover it would be kind if you could create a Jira sub-task  
>>>>> of https://issues.apache.org/jira/browse/OFBIZ-2330  (check before
>>>>> if a sub-task for this error does not exist). If you are not  
>>>>> sure  how to create a Jira issue please have a look before at
>>>>> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
>>>>> at the end of the current error message which is something like
>>>>>  Found URL parameter [partyId] passed to secure (https) request- 
>>>>> map  with uri [searchorders] with an event that calls service
>>>>> [findOrders]; this is not allowed for security reasons! The data   
>>>>> should be encrypted by making it part of the request body (a form
>>>>> field) instead of the request URL.; In session   
>>>>> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be   
>>>>> changed using the
>>>>> service.http.parameters.require.encrypted property in the   
>>>>> url.properties file
>>>>>
>>>>> What do you think ?
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>
>>>
>>
>


Re: Securing URLs in Freemarker templates

Posted by David E Jones <da...@hotwaxmedia.com>.
Here is an example of one place in an FTL file that does this (and  
there are a few other examples around):

${StringUtil.wrapString(productPromo.promoText?if_exists)}

The basic rule is very simple: if it is a java.lang.String object then  
it will be HTML/XML encoded in FTL templates! If you don't want the  
encode, then make it not a String.

-David


On Apr 22, 2009, at 4:06 PM, Jacques Le Roux wrote:

> I have added more information to the error message at r767688 for  
> the trunk and r767689 for R9.04.
>
> BTW, we now have <br> instead of real new line in error messages, is  
> there not a quick fix for that ?
>
> Jacques
>
> From: "Jacques Le Roux" <ja...@les7arts.com>
>> From: "David E Jones" <da...@hotwaxmedia.com>
>>> This practice is a bad one in general, and is usually used to   
>>> implement something like going back to a common page that is  
>>> better  done with newer framework features like the last-view  
>>> stuff in the  controller.xml file.
>>> What's worse is that using these sorts of things often introduces   
>>> security vulnerabilities because it is VERY easy to spoof  
>>> parameters  and then they are treated like internal data (and  
>>> that's aside from  the open URL versus encrypted form field issue).
>>> So yes, it would be best to get rid of all of these. Unfortunately  
>>> it  will take some work... on the other hand it looks like a lot  
>>> of the  code that uses those things is pretty messy and it will be  
>>> a huge  value add to clean them up (ie make the code easier to  
>>> maintain,  customize, reuse, etc).
>> And what about enhancing the error message ?
>> Jacques
>>> -David
>>> On Apr 21, 2009, at 10:25 AM, Jacques Le Roux wrote:
>>>> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260  
>>>> which  were ending as a mess and opened a Jira task with already 3
>>>> sub-tasks (taken from OFBIZ-2260)
>>>>
>>>> There are also some exceptions like we found in the widget part   
>>>> (strings with dynamic params names and value), see for instance
>>>> OFBIZ-2332.
>>>>
>>>> So at this stage we are caught, we don't accept such URLs but  
>>>> there  are hard to change. And unfortunately this scheme is pretty
>>>> often used
>>>> 6 ${paramString}</...@ofbizUrl>"
>>>> 26 ${paramList}</...@ofbizUrl>"
>>>> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
>>>> I'm not sure there are no other cases.
>>>> I believe we should think about a solution for all these   
>>>> exceptions... Maybe rewriting upstream as David already suggested.
>>>>
>>>> Also, in order to get more feedbacks, I'd like to add
>>>>  "Moreover it would be kind if you could create a Jira sub-task  
>>>> of https://issues.apache.org/jira/browse/OFBIZ-2330  (check before
>>>> if a sub-task for this error does not exist). If you are not  
>>>> sure  how to create a Jira issue please have a look before at
>>>> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
>>>> at the end of the current error message which is something like
>>>>  Found URL parameter [partyId] passed to secure (https) request- 
>>>> map  with uri [searchorders] with an event that calls service
>>>> [findOrders]; this is not allowed for security reasons! The data   
>>>> should be encrypted by making it part of the request body (a form
>>>> field) instead of the request URL.; In session   
>>>> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be   
>>>> changed using the
>>>> service.http.parameters.require.encrypted property in the   
>>>> url.properties file
>>>>
>>>> What do you think ?
>>>>
>>>> Jacques
>>>>
>>>>
>>>
>>
>


Re: Securing URLs in Freemarker templates

Posted by Jacques Le Roux <ja...@les7arts.com>.
I have added more information to the error message at r767688 for the trunk and r767689 for R9.04.

BTW, we now have <br> instead of real new line in error messages, is there not a quick fix for that ?

Jacques

From: "Jacques Le Roux" <ja...@les7arts.com>
> From: "David E Jones" <da...@hotwaxmedia.com>
>> This practice is a bad one in general, and is usually used to  
>> implement something like going back to a common page that is better  
>> done with newer framework features like the last-view stuff in the  
>> controller.xml file.
>> 
>> What's worse is that using these sorts of things often introduces  
>> security vulnerabilities because it is VERY easy to spoof parameters  
>> and then they are treated like internal data (and that's aside from  
>> the open URL versus encrypted form field issue).
>> 
>> So yes, it would be best to get rid of all of these. Unfortunately it  
>> will take some work... on the other hand it looks like a lot of the  
>> code that uses those things is pretty messy and it will be a huge  
>> value add to clean them up (ie make the code easier to maintain,  
>> customize, reuse, etc).
> 
> And what about enhancing the error message ?
> 
> Jacques
> 
>> -David
>> 
>> 
>> On Apr 21, 2009, at 10:25 AM, Jacques Le Roux wrote:
>> 
>>> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which  
>>> were ending as a mess and opened a Jira task with already 3
>>> sub-tasks (taken from OFBIZ-2260)
>>>
>>> There are also some exceptions like we found in the widget part  
>>> (strings with dynamic params names and value), see for instance
>>> OFBIZ-2332.
>>>
>>> So at this stage we are caught, we don't accept such URLs but there  
>>> are hard to change. And unfortunately this scheme is pretty
>>> often used
>>> 6 ${paramString}</...@ofbizUrl>"
>>> 26 ${paramList}</...@ofbizUrl>"
>>> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
>>> I'm not sure there are no other cases.
>>> I believe we should think about a solution for all these  
>>> exceptions... Maybe rewriting upstream as David already suggested.
>>>
>>> Also, in order to get more feedbacks, I'd like to add
>>>   "Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 
>>>  (check before
>>> if a sub-task for this error does not exist). If you are not sure  
>>> how to create a Jira issue please have a look before at
>>> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
>>> at the end of the current error message which is something like
>>>   Found URL parameter [partyId] passed to secure (https) request-map  
>>> with uri [searchorders] with an event that calls service
>>> [findOrders]; this is not allowed for security reasons! The data  
>>> should be encrypted by making it part of the request body (a form
>>> field) instead of the request URL.; In session  
>>> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be  
>>> changed using the
>>> service.http.parameters.require.encrypted property in the  
>>> url.properties file
>>>
>>> What do you think ?
>>>
>>> Jacques
>>>
>>>
>>
>


Re: Securing URLs in Freemarker templates

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "David E Jones" <da...@hotwaxmedia.com>
> This practice is a bad one in general, and is usually used to  
> implement something like going back to a common page that is better  
> done with newer framework features like the last-view stuff in the  
> controller.xml file.
> 
> What's worse is that using these sorts of things often introduces  
> security vulnerabilities because it is VERY easy to spoof parameters  
> and then they are treated like internal data (and that's aside from  
> the open URL versus encrypted form field issue).
> 
> So yes, it would be best to get rid of all of these. Unfortunately it  
> will take some work... on the other hand it looks like a lot of the  
> code that uses those things is pretty messy and it will be a huge  
> value add to clean them up (ie make the code easier to maintain,  
> customize, reuse, etc).

And what about enhancing the error message ?

Jacques
 
> -David
> 
> 
> On Apr 21, 2009, at 10:25 AM, Jacques Le Roux wrote:
> 
>> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which  
>> were ending as a mess and opened a Jira task with already 3
>> sub-tasks (taken from OFBIZ-2260)
>>
>> There are also some exceptions like we found in the widget part  
>> (strings with dynamic params names and value), see for instance
>> OFBIZ-2332.
>>
>> So at this stage we are caught, we don't accept such URLs but there  
>> are hard to change. And unfortunately this scheme is pretty
>> often used
>> 6 ${paramString}</...@ofbizUrl>"
>> 26 ${paramList}</...@ofbizUrl>"
>> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
>> I'm not sure there are no other cases.
>> I believe we should think about a solution for all these  
>> exceptions... Maybe rewriting upstream as David already suggested.
>>
>> Also, in order to get more feedbacks, I'd like to add
>>   "Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 
>>  (check before
>> if a sub-task for this error does not exist). If you are not sure  
>> how to create a Jira issue please have a look before at
>> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
>> at the end of the current error message which is something like
>>   Found URL parameter [partyId] passed to secure (https) request-map  
>> with uri [searchorders] with an event that calls service
>> [findOrders]; this is not allowed for security reasons! The data  
>> should be encrypted by making it part of the request body (a form
>> field) instead of the request URL.; In session  
>> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be  
>> changed using the
>> service.http.parameters.require.encrypted property in the  
>> url.properties file
>>
>> What do you think ?
>>
>> Jacques
>>
>>
>


Re: Securing URLs in Freemarker templates

Posted by David E Jones <da...@hotwaxmedia.com>.
This practice is a bad one in general, and is usually used to  
implement something like going back to a common page that is better  
done with newer framework features like the last-view stuff in the  
controller.xml file.

What's worse is that using these sorts of things often introduces  
security vulnerabilities because it is VERY easy to spoof parameters  
and then they are treated like internal data (and that's aside from  
the open URL versus encrypted form field issue).

So yes, it would be best to get rid of all of these. Unfortunately it  
will take some work... on the other hand it looks like a lot of the  
code that uses those things is pretty messy and it will be a huge  
value add to clean them up (ie make the code easier to maintain,  
customize, reuse, etc).

-David


On Apr 21, 2009, at 10:25 AM, Jacques Le Roux wrote:

> I have closed https://issues.apache.org/jira/browse/OFBIZ-2260 which  
> were ending as a mess and opened a Jira task with already 3
> sub-tasks (taken from OFBIZ-2260)
>
> There are also some exceptions like we found in the widget part  
> (strings with dynamic params names and value), see for instance
> OFBIZ-2332.
>
> So at this stage we are caught, we don't accept such URLs but there  
> are hard to change. And unfortunately this scheme is pretty
> often used
> 6 ${paramString}</...@ofbizUrl>"
> 26 ${paramList}</...@ofbizUrl>"
> 4 ${parameters.targetRequestUri}</...@ofbizUrl>"
> I'm not sure there are no other cases.
> I believe we should think about a solution for all these  
> exceptions... Maybe rewriting upstream as David already suggested.
>
> Also, in order to get more feedbacks, I'd like to add
>   "Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 
>  (check before
> if a sub-task for this error does not exist). If you are not sure  
> how to create a Jira issue please have a look before at
> http://docs.ofbiz.org/x/r. Thank you in advance for your help"
> at the end of the current error message which is something like
>   Found URL parameter [partyId] passed to secure (https) request-map  
> with uri [searchorders] with an event that calls service
> [findOrders]; this is not allowed for security reasons! The data  
> should be encrypted by making it part of the request body (a form
> field) instead of the request URL.; In session  
> [DF1819F1BFDCDFE831FD1ED3B5B2FE88.jvm1]; Note that this can be  
> changed using the
> service.http.parameters.require.encrypted property in the  
> url.properties file
>
> What do you think ?
>
> Jacques
>
>