I initially posted this to the security group, but there's precedent that MITM vulnerabilities can be public, so they told me to repost here...

The Virtual Merchant module posts credit card numbers and other sensitive information to https://www.myvirtualmerchant.com/VirtualMerchant/process.do

However, prior to this it disable SSL peer verification by calling curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, 0); This makes the module susceptible to a man-in-the-middle attack.

Instead, CURLOPT_SSL_VERIFYPEER should be enabled, and documentation should be provided to help users in case verification fails- see the documentation in the commerce_paypal_api_request() function as an example of a better implementation.

Additionally, CURLOPT_FOLLOWLOCATION should probably not be enabled. For instance, the connection could be redirected to an unsecured HTTP URL.

CommentFileSizeAuthor
#1 virtualmerchant-SSLcert-2189867-1.patch582 byteslaughnan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

laughnan’s picture

laughnan
he/him/his
CreditAttribution: laughnan commented
Assigned: Unassigned » laughnan
FileSize
virtualmerchant-SSLcert-2189867-1.patch582 bytes

Dane -

Did you mean this for the 6.x-1.0 version? I don't have any current D6 sites running, but drafted a quick patch with your suggested adjustment. Additional fixes might be to 1) add in some module help text?

Also for CURLOPT_FOLLOWLOCATION, the current module code has this disabled. Did you mean to enable it?

-- Alex

laughnan’s picture

laughnan
he/him/his
CreditAttribution: laughnan commented
Status: Active » Needs review
Dane Powell’s picture

Dane Powell CreditAttribution: Dane Powell commented

Hey Alex- that's the fix. I don't know why I thought FOLLOWLOCATION was enabled, maybe I was looking at old code or something.

So yeah, that's a good start, and then maybe just a little bit of documentation. Commerce Paypal has this to say:

<?php
  // Commerce PayPal requires SSL peer verification, which may prevent out of
  // date servers from successfully processing API requests. If you get an error
  // related to peer verification, you may need to download the CA certificate
  // bundle file from http://curl.haxx.se/docs/caextract.html, place it in a
  // safe location on your web server, and update your settings.php to set the
  // commerce_paypal_cacert variable to contain the absolute path of the file.
  // Alternately, you may be able to update your php.ini to point to the file
  // with the curl.cainfo setting.
?>

Something like that would probably be good to put in the README.

laughnan’s picture

laughnan
he/him/his
CreditAttribution: laughnan commented

Thanks for that thought, Dane. I'll work something like that into a README.txt file in the very near future.

  • laughnan committed 8f0dd92 on 7.x-1.x 
    Fixed SSL potential for MITM attack via #2189867
laughnan’s picture

laughnan
he/him/his
CreditAttribution: laughnan commented
Status: Needs review » Closed (fixed)