Closed Bug 753550 Opened 12 years ago Closed 12 years ago

Marketplace does not verify SSL cert when talking to PayPal

Categories

(Marketplace Graveyard :: Security, defect, P3)

Product:
Marketplace Graveyard
Component:
Security
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2012-05-24

People

(Reporter: clouserw, Assigned: kumar)

References

Details

(Keywords: sec-high)

On Tue, May 1, 2012 at 9:43 AM, Subodh Iyengar<mani.subodh@gmail.com> wrote: > Scope of the vulnerability: > We found a vulnerability in Zamboni, (the addons.mozilla.com store code > https://github.com/mozilla/zamboni). Zamboni uses paypal IPN for payment > processing, however it is open to a Man in the middle (MITM) attack in which > an MITM can spoof a "VERIFIED" payment message from paypal forcing zamboni > to accept an invalid payment. > > Details: > Zamboni uses Paypal IPN to process payments (for apps). The code for the IPN > sits in apps/paypal. During the IPN post-back procedure however, zamboni > connects to paypal using urllib, which does not do server certificate > validation. > > This can be seen in 2 code snippets: > 1. In apps/paypal/views.py > > with statsd.timer('paypal.validate-ipn'): > paypal_response = urllib2.urlopen(settings.PAYPAL_CGI_URL, > data, 20).readline() > post, transactions = _parse(post) > # If paypal doesn't like us, fail. > if paypal_response != 'VERIFIED': > msg = ("Expecting 'VERIFIED' from PayPal, got '%s'. " > "Failing." % paypal_response) > _log_error_with_data(msg, post) > return http.HttpResponseForbidden('Invalid confirmation') > > This is an unverified request and an MITM can easily spoof the VERIFIED > response > > 2. In /apps/paypal/__init__.py > > request = urllib2.Request(url) > .... > .... > opener = urllib2.build_opener() > try: > with socket_timeout(10): > feeddata = opener.open(request, _nvp_dump(paypal_data)).read() > except AuthError, error: > paypal_log.error('Authentication error: %s' % error) > raise > > This method also does not do any server validation. > > Fix: > Use the python ssl module to communicate with IPN and verify the hostname on > the end entity certificate afterwards.
Assignee: nobody → amckay
Wil, what's the target milestone for this? is it asap?
(In reply to Kumar McMillan [:kumar] from comment #2) > Wil, what's the target milestone for this? is it asap? I don't think so. Just something to get on our radar to solve when someone (apparently Andy) gets a chance. :)
this is, for the most part a solved problem, isn't it? can't we use a current solution? -r
oh yeah, it's definitely a quick fix. There are just a lot of other quick fixes in the queue. Andy sent me a patch that he started so I can put it in whenever. I was just wondering where to juggle it among the other tickets in my queue.
Priority: -- → P3
Fixed: https://github.com/mozilla/zamboni/commit/9645b19b888141dab6c31199742a594ad6929cb7 https://github.com/mozilla/zamboni/commit/d16c073924bfb8f6c28228e8b7a2d6b7256c71e8 The certifi module was also added as part of verifying SSL for in-app posts (bug 753560): https://github.com/mozilla/zamboni-lib/commit/800d5e3aa991f8cc9de7957f9c3379a22dd3bc7f The PayPal cert (and any other SSL cert) is now verified using the Mozilla CA bundle.
Assignee: amckay → kumar.mcmillan
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2012-05-24
Attachment #626613 - Attachment description: Web Bounty Nomination → Web Bounty Awarded 1000
Attachment #626613 - Attachment description: Web Bounty Awarded 1000 → Web Bounty Awarded $1000
Keywords: sec-high
Can we make this bug public now that it is fixed.
(In reply to mani subodh from comment #8) > Can we make this bug public now that it is fixed. Sure, done.
Group: client-services-security
Flags: sec-bounty+
Attachment #626613 - Attachment description: Web Bounty Awarded $1000 → Web Bounty Awarded $1000 [paid]
Attachment #626613 - Attachment description: Web Bounty Awarded $1000 [paid] → mani.subodh@gmail.com,1000,2012-05-01, 2012-06-04,2012-05-21,true
Attachment #626613 - Attachment filename: bugbountynom.txt → bugbounty.data
You need to log in before you can comment on or make changes to this bug.