moneris / moneris-gateway-api-php Goto Github PK

@MonerisSolutions
I find a critical bug within mpgResponse class.

The XML parser "failed" on identifying special XML entity character '&'. For example, we expect

<ACSUrl>https://host/path?item1=1&amp;item2=2</ACSUrl>

to be parsed as:

['ACSUrl' => 'https://host/path?item1=1&item2=2']

However, the actual outcome is:

['ACSUrl' => 'item2=2']

The cause of this bug is within method mpgResponse::characterHandler($parser, $data)

Due to '&' as an special XML entity character, the 'data node':

https://host/path?item1=1&item2=2

is tokenized into 3 separate fields:

https://host/path?item1=1
&
item2=2

It means mpgResponse::characterHandler would be invoked three times by php xml parser although the currentTag remain unchanged across these 3 times function calls.

Near the end of mpgResponse::characterHandler, there is a statement

$this->responseData[$this->currentTag] = $data;

So, the later tokenized string will overwrite the previous one. This explain the bug.
In fact, all 5 special xml entity characters will cause this bug.

Thus, I propose to concatenate the tokenized data string instead

$this->responseData[$this->currentTag]  .= $data;

The pdf documentation says that expiry_date is the field to use but that does not work, it should be expdate as in the code samples.

The class name mpgResponse is not unique in PHP and conflicts with a PEAR module used for testing naming conventions. Inclusion of a name space with the classes would eliminate the potential for this conflict.

To ensure alignment with industry best practices, every class and function contained within should have proper inline documentation using the PHPDoc format.

At the end of the method, we can see that the xml result is concatenate to the this->xmlString without clearing it. Therefore, if we call the method n times, it will generate n xmlString

I got the response like this from API:
[responseData] => Array
(
[ReceiptId] => 52-20170823164249
[ReferenceNum] => 664071750010010080
[ResponseCode] => 483
[ISO] => N7
[AuthCode] => 000000
[TransTime] => 12:42:49
[TransDate] => 2017-08-23
[TransType] => 00
[Complete] => true
[Message] => CALL FOR * AUTHORIZATION =
[TransAmount] => 3.49
[CardType] => V
[TransID] => 11-0_119
[TimedOut] => false
[Ticket] => null
[AvsResultCode] => Z
[ITDResponse] => null
[CvdResultCode] => 1N
[IsVisaDebit] => false
)
The message "CALL FOR * AUTHORIZATION =" is meaningless for the customer. How can I get detail failed reason for customer?

The file mpgClasses.php throws an error on PHP 8+ when count() is being passed a null variable. (lines 2619, 2604, etc..)

From the PHP docs:

count() will now throw [TypeError] on invalid countable types passed to the value parameter. (docs)

The Drupal commerce_moneris module needs to rely on a third party fork of this code because it is not set up with composer. This is problematic especially given the sensitive nature of this library. Please set up this repository so that developers can include it as a dependency via composer.

The PHP4 constructor style is deprecated in PHP7 and will be removed in PHP7.1

Please consider at least a PHP7.X compliant branch.

There are more than one class in mpgClasses.php that's why I am unable to use it in laravel framework any suggestion How I can use ?

Why was this changed? e4b0f1f#diff-6bbb785d66236916834b0d9dda64c5c1e6dad06784b78290e65311e7a14dceb6

When I build my project, I pull this one in as a submodule and now I get the debug on by default. Is there a better way to include your code in mine?

https://github.com/Moneris/eCommerce-Unified-API-PHP/blob/ea6b37d5df38b5d3fd04c198355ba2b8cef1793c/mpgClasses.php#L45

Is this not the production release of this library? In a previous commit, this default value of debug was changed from false to true...

https://github.com/Moneris/eCommerce-Unified-API-PHP/blob/3cd3f0bd5a92432c1b4f9727d1ca6334786d9066/mpgClasses.php#L20

1. What does "NA" refer to here? (North America? New API?)
2. How does this series of version numbers relate to the older 2015 version of mpgClasses.php which has a higher number? (like this: 'API_VERSION' =>'PHP - 2.5.6',)
3. Would it be correct to assume that the ~2015 version of mpgClasses.php was the starting point of this repo in 2016, and that a new numbering system was adopted at that time?

This API package needs a ton of work.

• There are no standards followed here for naming anything
• There's no code documentation anywhere on any method, and barely any for variables
• There's stuff all over the place
• There's formatting inconsistencies everywhere

I mean the damn file is almost 5000 lines long!

Who does the code review for this?

I'm worried about implementing this in anything production related.

Hi,
I am a Magento developer. I have used your code to integrate my Magento payment with Moneris.

When I use type "purchasecorrection", I see the response " Message = sendMessage(): Merchant - Could not initialize db object for request".

Can you please tell me why I got the error and how to fix?

Thanks!

For developers intending to use this library, please note:

• Moneris has not been responding to issues reported on Github.
• There are many new and old open issues, without any acknowledgement that Moneris has even seen them.
• It appears that Moneris uses GitHub as a place to host the library, without using any of the other features that the GitHub community would expect.
• I have tried to give feedback to Moneris about this, through the various methods recommended on their main website and developer portal. They redirected me many times to various different email addresses and phone numbers, without reaching anyone appropriate after 7 weeks.
• Moneris does not have a working process for reporting security issues.

As a result, I recommend that developers treat this library like they would an abandoned one. Yes I have used it successfully for many years, and yes the code does get some updates from Moneris. But if anything does not work, or an update breaks something, or a security issue is found, it is likely that we will need to fix it ourselves.

For example, on Feb 29, 2024, Moneris turned on debug mode, which could easily break things in production and leak your secret API keys by outputting API raw request data to the user. It was reported 2 weeks later by user @rayr007 , and in May by me, yet in over 3 months Moneris has not responded or fixed the problem.

It is worth noting that Moneris also has .NET and Java libraries on GitHub, and that they have published the compiled .dll and .jar for these, without the source code. At least with the PHP version we can review code changes and fix them in our own copies.

To Moneris:
If you do see this message, please comment on this and any open issues and pull requests.

This logic $this > isMPI2 is meaningless, and the response value will likely be wrong or empty.
on line 1470, it should be $this->isMPI2

Split this one class into individual files and update constructors.

Would be nice to have this available via composer.

Code should be restructured in such a way that it follows modern PHP coding standards for object-oriented code and established industry best practices.

All classes should be broken out into individual files.

2 validations are missing in toXML_low():

private function toXML_low($template,$txnType)
{
	$xmlString = "";
	//FIX: This line should be added.
	if( !empty($this->level3data[$txnType]) && is_array($this->level3data[$txnType]) )
	{
		for($x=0;$x<count($this->level3data[$txnType]);$x++)
		{
			if($x>0)
			{
				$xmlString .="</$txnType><$txnType>";
			}
			$keys=array_keys($template);
			for($i=0; $i < count($keys);$i++)
			{
				$tag=$keys[$i];

				if(is_array($template[$keys[$i]]))
				{
					$data=$template[$tag];

					//FIX: should add !is_array
					if( !is_array($this->level3data[$tag]) || !count($this->level3data[$tag]) )
					{
						continue;
					}
					$beginTag="<$tag>";
					$endTag="</$tag>";