Hi Samuel,
I agree that it's an overkill. I guess because services give you more power so
need to be more secure.
The same person that, like you, doubted about the reason of checkSecureParameter() spoke
also about the possibility to "inject stuff using parameters"
Here is what he said (actually this was reported to me by a 3rd person but I
much trust them both)
"The other case I reported has more to do with people being able to inject
stuff using parameters. Because they are not always escaped. In
particular the case for the VIEW_INDEX parameter and alike view_size, view_index
often in combination with screen widget but not only"
Anyway what would you suggest? You know you can set
service.http.parameters.require.encrypted=N, is that not enough?
Jacques
Le 18/10/2019 à 16:48, Samuel a écrit :
Jacques,
I agree that sending sensible data in url is definitely not a good practice, but I think it is overkill to forbid all url parameters to achieve this
security goal.
Moreover if you don't use a service event in your request map you can access whatever url parameter you want, so I cannot see why service event is
so particular in this regards.
Again my use case is to access url parameters in a service like accessing
view_size, or view_index which is definitely not sensible information.
Samuel
On 18/10/2019 16:21, Jacques Le Roux wrote:
Samuel,
This was initiated by David (the founder of the project). It was not much discussed at the time. I guess he had a possible attack scenario in head,
or faced one.
As he commented in ServiceEventHandler::checkSecureParameter
// special case for security: if this is a request-map defined as secure in controller.xml then only accept body parameters coming in, ie don't
allow the insecure URL parameters
// NOTTODO: may want to allow parameters that map to entity PK fields to be in the URL, but that might be a big security hole since there are
certain security sensitive entities that are made of only PK fields, or that only need PK fields to function (like UserLoginSecurityGroup)
// NOTTODO: we could allow URL parameters when it is not a POST (ie when !request.getMethod().equalsIgnoreCase("POST")), but that would open a
security hole where sensitive parameters can be passed on the URL in a GET/etc and bypass this security constraint
This article is related:
https://www.owasp.org/index.php/Information_exposure_through_query_strings_in_url
Here is an use case
https://www.fullcontact.com/blog/never-put-secrets-urls-query-parameters/
We could consider a CSRF attack. An attacker could create an email similar to the one we use to allow users to change passwords. In the fake email
an underneath URL could be
.../partymgr/control/ProfileAddUserLoginToSecurityGroup?userLoginId=ExistingEcommerceUser&partyId=ExistingEcommerceUser&AddUserLoginSecurityGroup=FULLADMIN
I let you imagine more :)
Jacques
Le 18/10/2019 à 11:28, Samuel a écrit :
Hi Jacques,
thanks for your detail and quick answer !
I still can't see the point with this check :s What kind of attack this check is protecting us ? could you describe an attack scenario where this
check is a good protection ?
My use case is to be able to access url parameters in an event service, I see that I can bypass the check with
`service.http.parameters.require.encrypted` property but still I really want to understand the point with this check ;)
Samuel
On 18/10/2019 10:48, Jacques Le Roux wrote:
Hi Samuel,
It started with http://svn.apache.org/viewvc?view=revision&revision=764286
Then I did http://svn.apache.org/viewvc?view=revision&revision=767688
Then I created OFBIZ-2330 after OFBIZ-2332, OFBIZ-2260, OFBIZ-2256
About removing, there are still few cases popping up. What is your case? Is it
relevant?
You are not the 1st one to question the security aspect, I commented that here:
https://s.apache.org/4z2bt
Thanks
Jacques
Le 18/10/2019 à 10:08, Samuel a écrit :
Hi,
recently I run against this check method which throw me an error to prevent me accessing url parameters from a service. Error message mentions a
security reason to forbid accessing url parameters but I definitely don't get this, so could you explain to me the "security" reason ? or could
we simply remove this check ?
Samuel
PS: I've also checked mentionned jira issue https://issues.apache.org/jira/browse/OFBIZ-2330, but this didn't help me understanding the
"security" reason