Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 0.6.8

Request #3815 Authorize.Net driver doesn't take advantage of MD5 hash
Submitted: 2005-03-15 00:12 UTC
From: jausions Assigned: jstump
Status: Closed Package: Payment_Process
PHP Version: Irrelevant OS: Irrelevant
Roadmaps: (Not assigned)    
Subscription  


 [2005-03-15 00:12 UTC] jausions
Description: ------------ The implementation of the Authorize.Net driver doesn't seem to take advantage of the MD5 hash security key. This is important to protect against DNS hacking. I'll try to write a patch for that... -Philippe

Comments

 [2005-03-16 23:27 UTC] jausions
This fix relies on bug #3854 being fixed. In Payment_Process_AuthorizeNet::process() <?php $response = &Payment_Process_Result::factory($this->_driver, $this->_responseBody); if (!PEAR::isError($response)) { $response->_request = & $this; $response->parse(); // --- new code // Check the response is legitimate by matching MD5 hashes if (!empty($this->_options['md5Value'])) { $hash = md5($this->_options['md5Value'] . $this->login . $response->transactionId . $response->amount); if ($hash != $response->md5Hash) { $response =& PEAR::raiseError('Illegitimate response from payment gateway.'); } } // -- end new code } ?> I have worked so much on that class... I don't have a separate patch file (at least for now) -Philippe
 [2005-03-23 17:58 UTC] jstump
I'll implement this once I talk to Ian about how to best handle bug 3854. I agree this feature should definitely be built into the AN driver and I plan on doing it. Obviously this should only be ran if the code has specified a md5Value in the options (which your code reflects - just restating the obvious for my own benefit).
 [2005-03-23 18:33 UTC] jausions
FYI: I have a better version which allow the changing of MD5 value without kicking out transaction in the process. This is because it takes time between changing the value in the Authorize.Net account and in the scripts. IMHO the MD5 value should be made mandatory, as this is added security. That would BC break, but security is more important in this case. I've done some improvments over the code posted here, so don't use as is to not waste your time... -Philippe
 [2005-03-23 19:13 UTC] jstump
Thank you for taking the time to write to us, but this is not a bug. After more thought Ian and I agree that this is outside of the scope of the AN driver. The purpose of Payment_Process is to offer a thin layer for CC processing gateways. In reality our code should give you a way for you to access this information in your user code. The problem with making anything mandatory is we would have to make it mandatory in ALL of the PP drivers. This would be akin to making views mandatory in DB, despite the fact that not all DB's support views.
 [2005-03-23 20:56 UTC] jausions
humm, sorry Joe, but I've got to disagree... not implementing this additional check is the same as adopting the Microsoft's philosophy to not close security hole by default. When it comes to security and payment information, I don't think we can afford to skip it. This is a very specific check for AN and not necessary of all PP processors. Each PP processors have different options particular to that payment gateway. We don't have to make the md5Value a mandatory setup value, but I would very strongly recommend it. The factory method can simply refuse to return a processor object if the processor isn't configured properly. Without that check you are also unable to verify that you are communicating with the actual AN gateway. This is especially important in the callback method (i.e. asynchornous notification, yes, with AIM) You have to at least ensure that you are not returning bogus information to the user. Puting this outside the scope of PP means it would have to be done by the user. I think that method would even more differentiate the PP processor from each other. If the PP AN processor does it automatically for the user than, this is even more abstraction gained... -Philippe
 [2005-06-23 14:23 UTC] jausions
Please use: http://www.11abacus.com/cvs/pear/Payment_Process.jausions.patch to correct this bug. -Philippe
 [2005-07-28 02:59 UTC] jstump
Thank you for your bug report. This issue has been fixed in the latest released version of the package, which you can download at http://pear.php.net/get/Payment_Process