Users - Hidden partyId - Security Risk?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Users - Hidden partyId - Security Risk?

Vinay Agarwal

Hello,

 

While going through credit card entry ftl’s, I came across applications/party/webapp/partymgr/party/editcreditcard.ftl which contains the following line

<input type="hidden" name="partyId" value="${partyId}"/>

I could be missing something here, but it sure looks like a security risk to me. Granted that this ftl is probably designed to be used only for Party Manager part of Webtools and not for a “public” application, but even that is not a good thing from code reuse point of view.

 

Regards,

Vinay Agarwal


 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Vinay Agarwal

Another similar case in applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl which contains

        <input type="hidden" name="paymentMethodId" value="${paymentMethodId}">

And this application is designed for public use. What am I missing here?

 

Regards,

Vinay Agarwal

 

-----Original Message-----
From: Vinay Agarwal [mailto:[hidden email]]
Sent: Friday, February 10, 2006 8:17 AM
To: 'OFBiz Users / Usage Discussion'
Subject: Hidden partyId - Security Risk?

 

Hello,

 

While going through credit card entry ftl’s, I came across applications/party/webapp/partymgr/party/editcreditcard.ftl which contains the following line

<input type="hidden" name="partyId" value="${partyId}"/>

I could be missing something here, but it sure looks like a security risk to me. Granted that this ftl is probably designed to be used only for Party Manager part of Webtools and not for a “public” application, but even that is not a good thing from code reuse point of view.

 

Regards,

Vinay Agarwal


 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Andrew Sykes
Vinay,

As a hacker, which I'm sure you're not, what would you actually do with
that data?

On Fri, 2006-02-10 at 08:23 -0800, Vinay Agarwal wrote:

> Another similar case in
> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl
> which contains
>
>         <input type="hidden" name="paymentMethodId"
> value="${paymentMethodId}">
>
> And this application is designed for public use. What am I missing
> here?
>
>  
>
> Regards,
>
> Vinay Agarwal
>
>  
>
> -----Original Message-----
> From: Vinay Agarwal [mailto:[hidden email]]
> Sent: Friday, February 10, 2006 8:17 AM
> To: 'OFBiz Users / Usage Discussion'
> Subject: Hidden partyId - Security Risk?
>
>  
>
> Hello,
>
>  
>
> While going through credit card entry ftl’s, I came across
> applications/party/webapp/partymgr/party/editcreditcard.ftl which
> contains the following line
>
> <input type="hidden" name="partyId" value="${partyId}"/>
>
> I could be missing something here, but it sure looks like a security
> risk to me. Granted that this ftl is probably designed to be used only
> for Party Manager part of Webtools and not for a “public” application,
> but even that is not a good thing from code reuse point of view.
>
>  
>
> Regards,
>
> Vinay Agarwal
>
>
>  _______________________________________________
> Users mailing list
> [hidden email]
> http://lists.ofbiz.org/mailman/listinfo/users
--
Kind Regards
Andrew Sykes <[hidden email]>
Sykes Development Ltd
http://www.sykesdevelopment.com

 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Si Chen-2
In reply to this post by Vinay Agarwal
I think the back end services they call will verify the user and
permissions. If not, then it definitely would be a risk.

Si

Vinay Agarwal wrote:

> Another similar case in
> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl
> which contains
>
> <input type="hidden" name="paymentMethodId" value="${paymentMethodId}">
>
> And this application is designed for public use. What am I missing here?
>
> Regards,
>
> Vinay Agarwal
>
> -----Original Message-----
> *From:* Vinay Agarwal [mailto:[hidden email]]
> *Sent:* Friday, February 10, 2006 8:17 AM
> *To:* 'OFBiz Users / Usage Discussion'
> *Subject:* Hidden partyId - Security Risk?
>
> Hello,
>
> While going through credit card entry ftl’s, I came across
> applications/party/webapp/partymgr/party/editcreditcard.ftl which
> contains the following line
>
> <input type="hidden" name="partyId" value="${partyId}"/>
>
> I could be missing something here, but it sure looks like a security
> risk to me. Granted that this ftl is probably designed to be used only
> for Party Manager part of Webtools and not for a “public” application,
> but even that is not a good thing from code reuse point of view.
>
> 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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

BJ Freeman
In reply to this post by Vinay Agarwal
both of those are ofbiz Id's without the corresponding data, in the
database, this is useless information.
if some tried to post this to ofbiz, they would run into the
certificate, then User privileges, the Security before any information
would be revealed about the cc.
The information, since ver 3.1, is encrypted in the DB so it would be
difficult, or impossible to retrieve such data.

I believe it would pass a security audit.

Vinay Agarwal sent the following on 2/10/06 8:23 AM:

> Another similar case in
> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl which
> contains
>
>         <input type="hidden" name="paymentMethodId"
> value="${paymentMethodId}">
>
> And this application is designed for public use. What am I missing here?
>
>  
>
> Regards,
>
> Vinay Agarwal
>
>  
>
> -----Original Message-----
> From: Vinay Agarwal [mailto:[hidden email]]
> Sent: Friday, February 10, 2006 8:17 AM
> To: 'OFBiz Users / Usage Discussion'
> Subject: Hidden partyId - Security Risk?
>
>  
>
> Hello,
>
>  
>
> While going through credit card entry ftl's, I came across
> applications/party/webapp/partymgr/party/editcreditcard.ftl which contains
> the following line
>
> <input type="hidden" name="partyId" value="${partyId}"/>
>
> I could be missing something here, but it sure looks like a security risk to
> me. Granted that this ftl is probably designed to be used only for Party
> Manager part of Webtools and not for a "public" application, but even that
> is not a good thing from code reuse point of view.
>
>  
>
> 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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Vinay Agarwal

1.      What could be done with it?

Let’s take example of applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl file. One could save the html page to local disk. Then one could change the ${paymentMethodId} value using a text editor and post the page back. If the security checks don’t catch this, this will have effect of a user get ability to modify paymentMethods that do not belong to him/her.

2.      Does the security check catch it? I am not sure if it catches it. Here are the details.

updateCreditCard service is called which calls

ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");

This checks whether partyId in the incoming map matches the partyId of the userLogin. After that, it is calling

  if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {

This function is defined in framework/security/src/org/ofbiz/security/OFBizSecurity.java.

It checks for group permissions but I don’t know enough to tell whether it does or does not catch this.

 

Regards,

Vinay Agarwal

 

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On Behalf Of bjfree
Sent:
Friday, February 10, 2006 9:39 AM
To: OFBiz Users / Usage Discussion
Subject: Re: [OFBiz] Users - Hidden partyId - Security Risk?

 

both of those are ofbiz Id's without the corresponding data, in the

database, this is useless information.

if some tried to post this to ofbiz, they would run into the

certificate, then User privileges, the Security before any information

would be revealed about the cc.

The information, since ver 3.1, is encrypted in the DB so it would be

difficult, or impossible to retrieve such data.

 

I believe it would pass a security audit.

 

Vinay Agarwal sent the following on 2/10/06 8:23 AM:

> Another similar case in

> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl which

> contains

>

>         <input type="hidden" name="paymentMethodId"

> value="${paymentMethodId}">

>

> And this application is designed for public use. What am I missing here?

>

>

> Regards,

>

> Vinay Agarwal

>

>

> -----Original Message-----

> From: Vinay Agarwal [mailto:[hidden email]]

> Sent: Friday, February 10, 2006 8:17 AM

> To: 'OFBiz Users / Usage Discussion'

> Subject: Hidden partyId - Security Risk?

>

>

> Hello,

>

>

> While going through credit card entry ftl's, I came across

> applications/party/webapp/partymgr/party/editcreditcard.ftl which contains

> the following line

>

> <input type="hidden" name="partyId" value="${partyId}"/>

>

> I could be missing something here, but it sure looks like a security risk to

> me. Granted that this ftl is probably designed to be used only for Party

> Manager part of Webtools and not for a "public" application, but even that

> is not a good thing from code reuse point of view.

>

>

> 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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Si Chen-2
In reply to this post by Vinay Agarwal
It should catch it... why not hack the form? Modify the .ftl, put in a
different paymentMethodId string, and then try it?

We always think it should do this or that, but.... :)

Vinay Agarwal wrote:

> 1. What could be done with it?
>
> Let’s take example of
> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl
> file. One could save the html page to local disk. Then one could
> change the ${paymentMethodId} value using a text editor and post the
> page back. If the security checks don’t catch this, this will have
> effect of a user get ability to modify paymentMethods that do not
> belong to him/her.
>
> 2. Does the security check catch it? I am not sure if it catches it.
> Here are the details.
>
> updateCreditCard service is called which calls
>
> ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context,
> result, "PAY_INFO", "_UPDATE");
>
> This checks whether partyId in the incoming map matches the partyId of
> the userLogin. After that, it is calling
>
> if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>
> This function is defined in
> framework/security/src/org/ofbiz/security/OFBizSecurity.java.
>
> It checks for group permissions but I don’t know enough to tell
> whether it does or does not catch this.
>
> Regards,
>
> Vinay Agarwal
>
> -----Original Message-----
> From: [hidden email]
> [mailto:[hidden email]] On Behalf Of bjfree
> Sent: Friday, February 10, 2006 9:39 AM
> To: OFBiz Users / Usage Discussion
> Subject: Re: [OFBiz] Users - Hidden partyId - Security Risk?
>
> both of those are ofbiz Id's without the corresponding data, in the
>
> database, this is useless information.
>
> if some tried to post this to ofbiz, they would run into the
>
> certificate, then User privileges, the Security before any information
>
> would be revealed about the cc.
>
> The information, since ver 3.1, is encrypted in the DB so it would be
>
> difficult, or impossible to retrieve such data.
>
> I believe it would pass a security audit.
>
> Vinay Agarwal sent the following on 2/10/06 8:23 AM:
>
>> Another similar case in
>
>> applications/ecommerce/webapp/ecommerce/customer/editcreditcard.ftl which
>
>> contains
>
>>
>
>> <input type="hidden" name="paymentMethodId"
>
>> value="${paymentMethodId}">
>
>>
>
>> And this application is designed for public use. What am I missing here?
>
>>
>
>>
>
>>
>
>> Regards,
>
>>
>
>> Vinay Agarwal
>
>>
>
>>
>
>>
>
>> -----Original Message-----
>
>> From: Vinay Agarwal [mailto:[hidden email]]
>
>> Sent: Friday, February 10, 2006 8:17 AM
>
>> To: 'OFBiz Users / Usage Discussion'
>
>> Subject: Hidden partyId - Security Risk?
>
>>
>
>>
>
>>
>
>> Hello,
>
>>
>
>>
>
>>
>
>> While going through credit card entry ftl's, I came across
>
>> applications/party/webapp/partymgr/party/editcreditcard.ftl which
> contains
>
>> the following line
>
>>
>
>> <input type="hidden" name="partyId" value="${partyId}"/>
>
>>
>
>> I could be missing something here, but it sure looks like a security
> risk to
>
>> me. Granted that this ftl is probably designed to be used only for Party
>
>> Manager part of Webtools and not for a "public" application, but even
> that
>
>> is not a good thing from code reuse point of view.
>
>>
>
>>
>
>>
>
>> 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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Andrew Sykes
In reply to this post by Vinay Agarwal
Vinay,

That's a fascinating scenario.

My gut response is that this wouldn't work, but like you, I can't be
sure.

It's certainly worth finding a definitive answer!
--
Kind Regards
Andrew Sykes <[hidden email]>
Sykes Development Ltd
http://www.sykesdevelopment.com

 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Vinay Agarwal

 

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 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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Si Chen-2
In reply to this post by Vinay Agarwal
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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Vinay Agarwal
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:[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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Si Chen-2
In reply to this post by Vinay Agarwal
I agree, but the other fix is also necessary: what if somebody found
some other way to access the system, ie via Web services?

Want to create a couple of JIRA issues for these, maybe post your
comments and mine?

Si

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:[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
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

David E. Jones
In reply to this post by Vinay Agarwal

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

smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Adrian Crum
I agree with David. I see user ID parameters in URLs in many different websites,
so it's not uncommon to handle sensitive data that way.

The services MUST check the parameters to make sure they are valid. OFBiz has
many security checks for the most part it is secure. If there is a hole
somewhere, then yes, we need to fix it. But, like David, I don't think we need
to re-write the entire application suite.

I've tried hacking into our OFBiz modifications by just changing parameters in
the URLs. As long as there is plenty of parameter checking going on at the
service level, our data remains secure.

David E. Jones wrote:

>
> 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
 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Sam-12
In reply to this post by Si Chen-2

I was very concerned about issues like these when we started down this
path so we decided to put a security proxy between the firewall and the
ofbiz server (http://www.modsecurity.org/index.php). mod_security will
catch remote form posts and many other issues. I don't have it working
properly yet but I will be glad to add the config and documentation to
the wiki when it is done.

SC


Si Chen wrote:

>I agree, but the other fix is also necessary: what if somebody found
>some other way to access the system, ie via Web services?
>
>Want to create a couple of JIRA issues for these, maybe post your
>comments and mine?
>
>Si
>
>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:[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
>  
>

 
_______________________________________________
Users mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/users
Reply | Threaded
Open this post in threaded view
|

Re: Users - Hidden partyId - Security Risk?

Vinay Agarwal
In reply to this post by Adrian Crum
I agree with all of you that all parameters must be checked to ensure
application security. What I am afraid of is that this may not be an
isolated issue. There may be other places that contain similar errors and it
may be repeated in future.

In order to determine the extent of the issues, I started looking at other
ftl's that have hidden field in them. There are about 240 ftl's like that. I
wasn't even able to rule out that the problem did not exist in the very
first ftl one from the search. Its details are below.

In both these cases, security check functions are being called but they
don't seem to check for the data that will be used for the operation.

Should we just expand the security check to include the data that will be
used? It seems to me that may solve a large number of potential issues.

Regards,
Vinay Agarwal

File:
applications/accounting/webapp/accounting/billingaccount/EditBillingAccount.
ftl

It has these lines right on the top
  <form name="billingform" method="post"
action="<@ofbizUrl>updateBillingAccount</@ofbizUrl>">
    <input type="hidden" name="billingAccountId"
value="${billingAccount.billingAccountId}">

Looking at the updateBillingAccount request in its controller.xml, it calls
        <check-permission permission="ACCOUNTING"
action="_UPDATE"><fail-property resource="AccountingUiLabels"
property="AccountingUpdateBillingAccountPermissionError"/></check-permission
>

But I don't think it checks for the billingAccountId. I wasn't able to test
because I couldn't find a way to create a billing account from unprivileged
account.

-----Original Message-----
From: [hidden email] [mailto:[hidden email]]
On Behalf Of Adrian Crum
Sent: Friday, February 10, 2006 5:30 PM
To: OFBiz Users / Usage Discussion
Subject: Re: [OFBiz] Users - Hidden partyId - Security Risk?

I agree with David. I see user ID parameters in URLs in many different
websites,
so it's not uncommon to handle sensitive data that way.

The services MUST check the parameters to make sure they are valid. OFBiz
has
many security checks for the most part it is secure. If there is a hole
somewhere, then yes, we need to fix it. But, like David, I don't think we
need
to re-write the entire application suite.

I've tried hacking into our OFBiz modifications by just changing parameters
in
the URLs. As long as there is plenty of parameter checking going on at the
service level, our data remains secure.

David E. Jones wrote:

>
> 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
>>

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