Re: Users - Hidden partyId - Security Risk?

Posted by David E. Jones on
URL: http://ofbiz.116.s1.nabble.com/Users-Hidden-partyId-Security-Risk-tp137366p137367.html


I VERY VERY VERY HIGHLY disagree with this approach... It is  
unreliable for security and putting things in the session causes all  
sort of flow problems in webapps. It is hard to avoid putting IDs in  
web pages or URLs and is generally a safe thing to do as long as the  
security for both displaying and changing data is sufficient, so that  
is what should be focused on...

The only way to properly control security of changes like this is to  
check things properly in the service implementations that process the  
incoming data.

It sounds like in your example that it is not checking to make sure  
that if the user has no special permission to make the changes that  
the partyId on the userLogin matches that on the record being  
changed, which should be considered a bug and the fix is to change  
that service to check this...

-David


On Feb 10, 2006, at 5:46 PM, Vinay Agarwal wrote:

> I think there is a general security risk with storing database  
> keys, or any
> "important" information, in form fields, hidden or not. Overall  
> security is
> much easier to achieve by not giving user access to this data than  
> to try to
> contain the contaminant input. In other words follow these two rules:
>
> 1. Not store any important information in user forms, instead save  
> them in
> session.
> 2. Save only the data in hidden user form that cannot impact  
> security if
> changed, e.g., sequence page id for multi-page data entry forms.
>
> So, I would simply recommend converting all 241 or so potentially  
> unsafe
> ftl's and corresponding event handlers to use session and instead  
> of hidden
> fields.
>
> Regards,
> Vinay Agarwal
>
>
> -----Original Message-----
> From: [hidden email] [mailto:users-
> [hidden email]]
> On Behalf Of Si Chen
> Sent: Friday, February 10, 2006 4:10 PM
> To: OFBiz Users / Usage Discussion
> Subject: Re: [OFBiz] Users - Hidden partyId - Security Risk?
>
> Vinay,
>
> My hunch is that:
> ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context,
> result, "PAY_INFO", "_UPDATE");
> is not enough. This checks if the partyId of the userLogin is the
> partyId in the context or if the userLogin has PAY_INFO_UPDATE.
>
> I think the security check there should be:
> 1. Does userLogin have PAY_INF_UPDATE? Yes -> good. This can be done
> with hasEntityPermission
> 2. If not (1), is the userLogin the partyId in the context AND the
> partyId of the PaymentMethod? Yes -> good.
>
> We may be missing the second part here...
>
> Want to try it? :)
>
> Si
>
> Vinay Agarwal wrote:
>
>> My testing did find problem with hidden paymentMethodId field that I
>> am describing below. In addition, there are 240 other ftl files that
>> contain hidden fields and may pose security risk although I have not
>> looked at anyone else.
>>
>> File: applications/ecommerce/webapp/ecommerce/customer/
>> editcreditcard.ftl
>>
>> Statement: <input type="hidden" name="paymentMethodId"
>> value="${paymentMethodId}">
>>
>> Theory:
>>
>> A hacked form may change the paymentMethodId and modify data that the
>> user does not have authorization for
>>
>> Method:
>>
>>    1. Ecommerce application, signed up as "firstuser" and added a
>>       credit card. Its paymentMethodId came out to be 10000.
>>    2. Logged out and signed up as "seconduser" and added a credit
>>       card. Its paymentMethodId came out to be 10001.
>>    3. Logged in as seconduser, clicked on update credit card. Saved
>>       the html page locally.
>>    4. Edited the saved html page
>>          1. Changed paymentMethodId from 10001 to 10000.
>>          2. Added http://localhost:8443 <http://localhost:8443/> to
>>             the action url.
>>    5. Expected result: firstuser and seconduser each has one  
>> credit card.
>>    6. Actual result: firstuser had no card and the second user had 2
>>       cards as seen on the profile page.
>>
>> Conclusion:
>>
>> A user is able to modify data that he is not authorized for.
>>
>> I would like to know if you can reproduce it. I can add it to Jira if
>> needed.
>>
>> Regards,
>>
>> Vinay Agarwal
>>
>> ---------------------------------------------------------------------
>> ---
>>
>>
>> _______________________________________________
>> Users mailing list
>> [hidden email]
>> http://lists.ofbiz.org/mailman/listinfo/users
>>
>
> _______________________________________________
> Users mailing list
> [hidden email]
> http://lists.ofbiz.org/mailman/listinfo/users
>
> _______________________________________________
> Users mailing list
> [hidden email]
> http://lists.ofbiz.org/mailman/listinfo/users

 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users