Flexible Return Authorization (RMA)

**chadlly2003** · 9 Jan 2020, 02:07 AM

I use the zencart module Flexible Return Authorization (RMA)
https://www.zen-cart.com/downloads.php?do=file&id=1692

The form seems to be working fine. Every once in awhile I get a syntax error. Not sure How to go about and fix this. If a customer puts a special characteristics in the form do i need to block that?

this is what the customer put in the form.

HTML Code:

Incorrect item from order 52394: ME-ARTR, Magnum Energy ME-ARTR, Magnum Advacnced Router. Quantity:1

Would like to exchange for correct item: ME-ARC50, MAGNUM ENERGY ME-ARC50, ADVANCED REMOTE DIGITAL LCD DISPLAY REMOTE PANEL WITH 50' CABLE. Quantity: 2

Please advise if this is possible.

error

HTML Code:

[08-Jan-2020 20:12:02 America/New_York] PHP Fatal error:  1064:You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CABLE. Quantity: 2



Please advise if this is possible.', '7', '12345678', now(' at line 1 :: insert into orders_status_history (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('Incorrect item from order 52394: ME-ARTR, Magnum Energy ME-ARTR, Magnum Advacnced Router. Quantity:1



Would like to exchange for correct item: ME-ARC50, MAGNUM ENERGY ME-ARC50, ADVANCED REMOTE DIGITAL LCD DISPLAY REMOTE PANEL WITH 50' CABLE. Quantity: 2



Please advise if this is possible.', '7', '12345678', now(), '1234567801082020', 'Get a Refund') ==> (as called by) /home/inverter/public_html/includes/templates/theme871/templates/tpl_returns_default.php on line 85 <== in /home/inverter/public_html/includes/classes/db/mysql/query_factory.php on line 171

I found where the issue is coming from in templates\tpl_returns_default.php on line 83

Code:

if (ORDER_COMMENTS_RMA_OPTION == 'true') {
$returnRMA = $orderID . $rma_number;
$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . $reason ."', '" . $autoRMA ."', '" . (int)$orderID ."', now(), '" . $returnRMA ."', '" . $action ."')");
}

**twitchtoo** · 9 Jan 2020, 02:16 AM

What are the properties of the comments field in your database? The ' will stop the code as special characters likely aren't permitted in that field.

**chadlly2003** · 9 Jan 2020, 02:32 AM

This is a snapshot of the database field.....

Name: Untitled-2.jpg
Views: 54
Size: 61.9 KB

**mc12345678** · 9 Jan 2020, 03:00 AM

Originally Posted by chadlly2003

I use the zencart module Flexible Return Authorization (RMA)
https://www.zen-cart.com/downloads.php?do=file&id=1692

The form seems to be working fine. Every once in awhile I get a syntax error. Not sure How to go about and fix this. If a customer puts a special characteristics in the form do i need to block that?

this is what the customer put in the form.

HTML Code:

Incorrect item from order 52394: ME-ARTR, Magnum Energy ME-ARTR, Magnum Advacnced Router. Quantity:1

Would like to exchange for correct item: ME-ARC50, MAGNUM ENERGY ME-ARC50, ADVANCED REMOTE DIGITAL LCD DISPLAY REMOTE PANEL WITH 50' CABLE. Quantity: 2

Please advise if this is possible.

error

HTML Code:

[08-Jan-2020 20:12:02 America/New_York] PHP Fatal error:  1064:You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CABLE. Quantity: 2



Please advise if this is possible.', '7', '12345678', now(' at line 1 :: insert into orders_status_history (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('Incorrect item from order 52394: ME-ARTR, Magnum Energy ME-ARTR, Magnum Advacnced Router. Quantity:1



Would like to exchange for correct item: ME-ARC50, MAGNUM ENERGY ME-ARC50, ADVANCED REMOTE DIGITAL LCD DISPLAY REMOTE PANEL WITH 50' CABLE. Quantity: 2



Please advise if this is possible.', '7', '12345678', now(), '1234567801082020', 'Get a Refund') ==> (as called by) /home/inverter/public_html/includes/templates/theme871/templates/tpl_returns_default.php on line 85 <== in /home/inverter/public_html/includes/classes/db/mysql/query_factory.php on line 171

I found where the issue is coming from in templates\tpl_returns_default.php on line 83

Code:

if (ORDER_COMMENTS_RMA_OPTION == 'true') {
$returnRMA = $orderID . $rma_number;
$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . $reason ."', '" . $autoRMA ."', '" . (int)$orderID ."', now(), '" . $returnRMA ."', '" . $action ."')");
}

The problem is clearly coming from the sql statement construction:

Code:

$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . $reason ."', '" . $autoRMA ."', '" . (int)$orderID ."', now(), '" . $returnRMA ."', '" . $action ."')");

Where the data being inserted is not escaped/prepared for database inserting. In this case, the single quote within the field causes an imbalance of quotes in the sql statement and could lead to security issues.

Data that is to be inserted into the database needs to be properly prepared either in statements leading to this query or within the query itself. Something like this would address that issue and allow addition of the associated comment. Personally I don't know how the "properties of the comments field" have anything to do with this simple statement that was entered as it wasn't an extended code such as might be seen in foreign languages that would use utf8mb4 type content.

Code:

$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . zen_db_input(zen_db_prepare_input($reason)) ."', '" . zen_db_input(zen_db_prepare_input($autoRMA)) ."', '" . (int)$orderID ."', now(), '" . zen_db_input(zen_db_prepare_input($returnRMA)) ."', '" . zen_db_input(zen_db_prepare_input($action)) ."')");

May have overdone one or the other prepare statements; however, it is unclear about the origin/need for sanitization for some of the values. If it is user enterable, then likely needs to be prepared to prevent malicious action.

There is another way to do something like this also:
Generate the sql statement and then bind the variable to the statement in the type the variable is expected:

Code:

$sql = $db->bindvars($sql, ':reason:', $reason, 'string');

Where the sql satatemnt would have:

Code:

(:reason:, :autoRMA:, etc...

And similar replacement done for each.

**chadlly2003** · 9 Jan 2020, 03:23 AM

This seems to solve the issue. My question is which is a better option to protect myself against malice actions

the first example or second

Code:

$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . zen_db_input(zen_db_prepare_input($reason)) ."', '" . zen_db_input(zen_db_prepare_input($autoRMA)) ."', '" . (int)$orderID ."', now(), '" . zen_db_input(zen_db_prepare_input($returnRMA)) ."', '" . zen_db_input(zen_db_prepare_input($action)) ."')");

**mc12345678** · 9 Jan 2020, 04:19 AM

Originally Posted by chadlly2003

This seems to solve the issue. My question is which is a better option to protect myself against malice actions

the first example or second

Code:

$db->Execute("insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values ('" . zen_db_input(zen_db_prepare_input($reason)) ."', '" . zen_db_input(zen_db_prepare_input($autoRMA)) ."', '" . (int)$orderID ."', now(), '" . zen_db_input(zen_db_prepare_input($returnRMA)) ."', '" . zen_db_input(zen_db_prepare_input($action)) ."')");

Well, for readability a little of a combination of both seems "better". Looking at the code that is called by the first option, it appears that the following would be a better choice:

Code:

$sql = "insert into " . TABLE_ORDERS_STATUS_HISTORY . " (comments, orders_status_id, orders_id, date_added, rma_number, action) values (:reason:, :autoRMA:, " . (int)$orderID .", now(), :returnRMA:, :action:)";

$sql = $db->bindVars($sql, ':reason:', zen_db_prepare_input($reason), 'string');
$sql = $db->bindVars($sql, ':autoRMA:', zen_db_prepare_input($autoRMA), 'string');
$sql = $db->bindVars($sql, ':returnRMA:', zen_prepare_input($returnRMA), 'string');
$sql = $db->bindVars($sql, ':action:', zen_db_prepare_input($action), 'string');
 
$db->Execute($sql);

My previous recommendation skipped the zen_db_prepare_input portion which strips slashes, then converts certain space characters to a space and replaces < and > (I think) with an underscore to prevent the inclusion of html type tags and then removes opening and closing spaces. After that the db->bindVars escapes characters to support insertion as a string. But, if there is a need for html tags to be included (or simple characters such as greater than and/or less than), then may need to remove the zen_db_prepare_input portion...