New error - Due to PHP update(?)

[Portman]

Hi,

Quite a while ago I created a module that allowed me to count stock qty on some items and not others... It worked fine. I discovered today that it is no longer working, so I must have done something to cause that, but I cannot for the life of me work out what it is - the only thing that I have done that MAY have caused the issue is to upgrade what version of PHP I was running ...

Firstly, I am currently on phoenix v1.0.8.20 and have PHP 8.1 is that ok?

Secondly here is the error I am getting and the code causing the error - can someone tell me what is the problem, as I can't work out what the error even means
Error:

Code: Select all

PHP Warning:  Undefined array key "pID" in /.../includes/hooks/admin/catalog/stockCount.php on line 15
PHP Fatal error:  Uncaught TypeError: Text::input(): Argument #1 ($s) must be of type string, null given, called in /.../includes/hooks/admin/catalog/stockCount.php on line 15 and defined in /.../includes/system/versioned/1.0.8.2/text.php:34
Stack trace:
#0 /.../includes/hooks/admin/catalog/stockCount.php(15): Text::input(NULL)
#1 /.../includes/system/versioned/1.0.8.1/hooks.php(150): hook_admin_catalog_stockCount->listen_injectDataForm(Array)
#2 /.../admin/includes/actions/catalog/views/new_product.php(220): hooks->cat('injectDataForm')
#3 /.../admin/catalog.php(48): require('/home/devcreat/...')
#4 {main}
  thrown in /.../includes/system/versioned/1.0.8.2/text.php on line 34
 PHP Deprecated:  Function strftime() is deprecated in /.../includes/system/versioned/1.0.8.3/date.php on line 45

Hook Code:

Code: Select all

<?php

	class hook_admin_catalog_stockCount {
		
	    function listen_updateProductAction() {
		$this->listen_productActionSave();
	}

    function listen_insertProductAction() {
		$this->listen_productActionSave();
	}
	
		function listen_injectDataForm() {
			
		    $products_id = Text::input($_GET['pID']);	
			$grab_val = $GLOBALS['db']->query("SELECT products_count_stock FROM products WHERE products_id = " . $products_id)->fetch_assoc();
			$product_count_stock = $grab_val['products_count_stock'];
			  $out_status = '0' == $product_count_stock;
			  $in_status = !$out_status;

?>
        <div class="form-group row" id="zCountStock">
          <label for="pCountStock" class="col-form-label col-sm-3 text-left text-sm-right"><?= TEXT_COUNT_STOCK_TITLE ?></label>
          <div class="col-sm-9">
		  
		   <div class="col-sm-9">
			<div class="custom-control custom-radio custom-control-inline">
			  <?= (new Tickable('product_count_stock', ['value' => '1', 'id' => 'inStatus', 'class' => 'custom-control-input'], 'radio'))->tick($in_status) ?>
			  <label class="custom-control-label" for="inStatus"><?= TEXT_COUNT_STOCK_YES ?></label>
			</div>
			<div class="custom-control custom-radio custom-control-inline">
			  <?= (new Tickable('product_count_stock', ['value' => '0', 'id' => 'outStatus', 'class' => 'custom-control-input'], 'radio'))->tick($out_status) ?>
			  <label class="custom-control-label" for="outStatus"><?= TEXT_COUNT_STOCK_NO ?></label>
			</div>
		  </div>
		 </div>
        </div>
<?php		
	}
	
    function listen_productActionSave() {
      global $products_id, $db;

      $sql_data_array = ['products_count_stock' => (int)$_POST['product_count_stock'] ?? 0];

      $db->perform('products', $sql_data_array, 'update', "products_id = '" . (int)$products_id . "'");
    }

}

[heatherbell]

Yes, PHP 8.1 will cause issues that need fixing like strftime() is deprecated, stick at 8.0
Guessing that the first warning is saying that pID is not existing or is NULL which PHP8.1 doesn't like so it gives the second Fatal Error.
Stick at 8.0. which is more forgiving, otherwise, just guessing, try:

Code: Select all

Text::input($_GET['pID'] ?? '');

which might catch the NULL and give an empty string instead.

[Portman]

Thanks, I'll just roll it back to 8.0... I'm surprised that this is the first error I have found, I changed it to 8.1 a few weeks even months ago.

[Portman]

HI Again @heatherbell

I rolled PHP back to 8.0... it did not fix the problem, so I applied what you suggested as a fix as well... this brought upa new error;

Code: Select all

PHP Warning:  DB: [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 '' at line 1 from <SELECT products_count_stock FROM products WHERE products_id = > in /.../includes/system/versioned/1.0.8.1/database_core.php on line 44

Any idea what is causing this?

[ecartz]

For security reasons (SQL injection!), that should be

Code: Select all

WHERE products_id = " . (int)$products_id)

[Portman]

Thanks for that ... It's working again now!
I assume the (int) stands for integer? what is the reason (if you can explain easily enough) that it is insecure without this?

[heatherbell]

I assume the (int) stands for integer? what is the reason (if you can explain easily enough) that it is insecure without this?

By casting $products_id to an integer, you're essentially sanitising the input, ensuring that it can only be interpreted as a numeric value. Any attempt to inject SQL code will fail because the input is not interpreted as SQL code but as a numeric value.