[RESOLVED] Basic PHP / MySQLi Prepared Statements Help

[Major_Disaster]

Hello,

I built a stock search using PHP / MySQL and it worked great. I then went about hardening it up and read about MySQLi. Being a newbie i am trying to use the Procedural Style (not the OO), but i can't get the following working:

		// Create a new mysqli object with database connection parameters
		$link = mysqli_connect('localhost', 'user', 'pass', 'db');

	if(mysqli_connect_errno()) {
		echo "Connection Failed: " . mysqli_connect_errno();
		echo mysqli_error();
	exit();
	}

	// Setup query
	$query = "SELECT * FROM `stock_search` WHERE `part_number` LIKE (?)";

	// Paramater to be bound
	$part_search = trim(filter_var($_POST['part-number'], FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES));

	// Get instance of statement
	$stmt = mysqli_stmt_init($link);

	// Prepare Query
	if(mysqli_stmt_prepare($stmt, $query)){

		/* Bind parameters
		s - string, b - boolean, i - int, etc */
		 mysqli_stmt_bind_param($stmt, "s", $part_search);

		/* Execute it */
		mysqli_stmt_execute($stmt);

		/* Bind results */
		mysqli_stmt_bind_result($stmt, $part_number, $manufacturer, $stock);

		/* Fetch the value */
		 mysqli_stmt_fetch($stmt);

		 $count = mysqli_num_rows($stmt);
		 echo $count;

		/* Close statement */
		 mysqli_stmt_close($stmt);
	}

	// Close Connection
	mysqli_close($link);

With the error, corresponding to the line: $count = mysqli_num_rows($stmt);

Warning: mysqli_num_rows() expects parameter 1 to be mysqli_result, object given

Any ideas where im going wrong? Im completely lost!

Thanks for any help

[bradgrafelman]

[man]mysqli_num_rows/man expects an object of type MySQLi_Result to be passed to it.

Since you're using prepared statements (something I don't think you really need, in this instance, since you don't appear to be re-using the statement... but perhaps that's a matter of preference), you'll want to use the [man]mysqli_stmt_num_rows/man function and pass it the MySQLi_STMT object you have.

[Major_Disaster]

Thanks a lot for the reply.

I inserted mysqli_stmt_num_rows() and tried a search - but got a result of 0 (when i should have had some hits). So im wondering, is there something wrong with my SQL? Specifically the LIKE part? I wanted to use:

$query = "SELECT * FROM `stock_search` WHERE `part_number` LIKE %?%";

But that throws an error.

With regards to prepared statements - im completely new to all this and worried about making rookie security mistakes. I read prepared statements helped prevent SQL injection so blindly tried using them. The statement will not be reused - so should i abandon the code?

I was following the prepared statement example here, should i switch to the "Basic Usage Example"?

Thanks a lot for the help

[Major_Disaster]

(Could've see the edit button?)

Just wanted to add, the reason i thought prepared statements were secure was becuase of the ? in the query, and then binding the parameters later. I don't actually know what a prepared statement is (ie the definition of one), but by abandoning my code above would i loose this protection?

[bradgrafelman]

Major_Disaster wrote:

So im wondering, is there something wrong with my SQL? Specifically the LIKE part?

I believe the problem is that the '%' wildcards need to be part of the parameter you bind to the variable, not in the SQL query being prepared. In other words, when you bind the variable, you'd bind something like:

'%' . $user_data . '%'

rather than placing the %'s in the SQL query string.

Major_Disaster wrote:

With regards to prepared statements - im completely new to all this and worried about making rookie security mistakes. I read prepared statements helped prevent SQL injection so blindly tried using them.

They help in that they automatically sanitize/escape the data being bound to the parameters in the statement. They offer no more security than raw queries where you manually sanitize/escape the data before placing it directly into the SQL query string.

In other words, your query string above could be written (with proper sanitization/escaping) like so:

$query = sprintf("SELECT * FROM `stock_search` WHERE `part_number` LIKE '%s'",
        mysqli_real_escape_string($link, $part_search)
);

(Note that the use of [man]sprintf/man certainly isn't required - I just like to use it when building SQL query strings as I find it helps keep the code cleaner and also allows you to do explicit type-casting for data sanitization when dealing with numeric data types.)

Major_Disaster wrote:

The statement will not be reused - so should i abandon the code?

Like I said above, that might be a matter of preference. I'm no SQL/DB guru, but in my personal opinion/experience, using prepared statements for queries that are only executed one time were a waste of typing/code space and/or resources. Writing the code to prepare a statement, bind my input variables, bind my output variables, execute the statement, etc. all seemed laborious as compared to just executing a raw SQL query string an fetching the results.

Are prepared statements worth learning and/or beneficial in certain situations? Of course! Do I think they are always more beneficial? Nope.

Major_Disaster wrote:

I was following the prepared statement example here, should i switch to the "Basic Usage Example"?

Eh... that's up to you. There are a couple of phrases/"facts" in that article that I disagree with (in intensities varying from mild to quite strong).

For example, the author of that article writes:

SECURITY!!!!!!!!:
Since the binary protocol in mysql has changed, and now supports prepared statements, this gives you an extremely secure method for executing queries. This completely elminated the possibility
of SQL Injections.

The elimination of SQL injection attacks can easily be accomplished using the mysql PHP extension provided that the programmer has properly educated himself/herself in the topic of security and SQL. He seems to imply that this wasn't possible until MySQLi was released, which is certainly false.

The thing that I don't like is that everyone seems to think that using prepared statements is so much better because you don't have to think about the security involved. That may be true, but I don't think that's beneficial at all. I still think the programmer should know why prepared statements are safer, e.g. what happens that makes them safe.

[Major_Disaster]

Once again thank you for your reply - i appreciate the expert support.

I tried playing with the wild card as you suggested, but this did not work, however I think i am going to leave the prepared statement and use the approach that you suggested.

Im just wondering, where you have:

$query = sprintf("SELECT * FROM `stock_search` WHERE `part_number` LIKE '%s'",
        mysqli_real_escape_string($link, $part_search)
); 

Could you please expand on what the %s is? Is it a placeholder for a variable like i had before, ie:

$part_search = trim(filter_var($_POST['part-number'], FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES));

Or is it (as i somewhat doubt) a placeholder that needs to be bound like before with the prepared statement?

I think i will read up a bit more on MySQLi and the procedural style. Thanks for the help

[Major_Disaster]

Oh, is the %s used to format the sprintf()? If so, where do i specify my $part_search?

[bradgrafelman]

Major_Disaster wrote:

Oh, is the %s used to format the sprintf()?

Yes, it's a parameter placeholder... specifically, it's a placeholder for a string parameter. The manual page for [man]sprintf/man has a list of these placeholders you'd use in the 'format' string.

Major_Disaster wrote:

If so, where do i specify my $part_search?

You specify the data as the (n+1)'th function parameter (+1 because the first parameter is the format string) for the sprintf() function. If it's the first (and/or only) parameter in the string (from left to right), then it goes in the 2nd (n+1, n=1) parameter of the sprintf() function. Again, refer to the manual page for [man]sprintf/man for coding examples.

[Major_Disaster]

Hello again, thanks for all the help. Ive been hacking away and this is what ive come up with. I would really appreciate it if you could pick it apart and point out any security flaws:

// Set blacklists
		$badwords = "/(bad|words)/i";
		$exploits = "/(content-type|bcc:|cc:|document.cookie|onclick|onload|javascript)/i";
		$bots = "/(Indy|Blaiz|Java|libwww-perl|Python|OutfoxBot|User-Agent|PycURL|AlphaServer|T8Abot|Syntryx|WinHttp|WebBandit|nicebot)/i";

	// Check for any bots
	if(preg_match($bots, $_SERVER['HTTP_USER_AGENT'])) {
		header("HTTP/1.0 404 Not Found"); 
		die('<h1>Spam bots are not welcome.</h1>');
	}

	// Get search input
	if(trim($_POST['part-number']) == '' ) {
		echo 'You didn\'t enter a <b>Part Number</b>.<br />';
		echo 'Please enter a new search query.';
		die();
	} else if (preg_match($badwords, trim($_POST['part-number'])) !== 0 || preg_match($exploits, trim($_POST['part-number'])) !== 0) {
		echo 'You entered a <b>Part Number</b> which contains unacceptable or explicit words.<br />';
		echo 'Please enter a new search query.';
		die();
	} else {
		$part_search = trim(filter_var($_POST['part-number'], FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES));
	}

	// Connect to the database
	$link = mysqli_connect('localhost', 'user', 'pass', 'db');

	// Check for any errors
	if(mysqli_connect_errno()) {
		printf("%s", mysqli_connect_error());
		exit();
	}

	// Escape characters in $part_search
	$part_search = mysqli_real_escape_string($link, $part_search);

	// Setup database query
	$query = sprintf("SELECT * FROM `stock_search` WHERE `part_number` LIKE '%s'", '%'.$part_search.'%');

	// Perform the search
	if($result = mysqli_query($link, $query)){

		// Count the number of results
		$number_results = mysqli_num_rows($result);

		// If no results are found
		if ($number_results == 0) {
			echo '<div id="center-800">';
			echo '<h2>No results were found for: "' . $part_search . '"</h2>';
			echo '<h3>Please <a href="/contact/">click here</a> to send us the enquiry or try a new search</h3>';
			echo '</div>';

		// Or, display the results that are found
		} else {
			echo '<div id="center-800">';
			echo '<h2>Search results for "' . $part_search . '"</h2>';
			echo '<h3>' . $number_results . ' matching parts found:</h3>';

// Display the table of results
				echo '<table class="stock-results">';
				echo '<tr>';
				echo '<th>RFQ</th>';
				echo '<th>Part Number</th>';
				echo '<th>Manufacturer</th>';
				echo '<th>Stock</th>';
				echo '<th>Quantity</th>';
				echo '<th>Delivery</th>';
				echo '</tr>';

			// Define base values for iteration
			$row_index = 0;
			$tab_index = 8;

			// Populate each row
			while ($rows = mysqli_fetch_array($result)) {

				// If there is a remainder, then the number is odd
				if($row_index % 2){
					$row_type = 'odd';
				} else {
					$row_type = 'even';
				}

				echo '<tr class="' . $row_type . '">';
				echo '<td><input type="checkbox" value="' . $row_index . '" name="id['. $row_index .']" tabindex="' . $tab_index++ .'"></td>';
				echo '<td><input type="hidden" value="' . $rows['part_number'] . '" name="part-number['. $row_index .']">' . $rows['part_number'] . '</td>';
				echo '<td><input type="hidden" value="' . $rows['manufacturer'] . '" name="manufacturer['. $row_index .']">' . $rows['manufacturer'] . '</td>';
				echo '<td class="stock"><input type="hidden" value="' . $rows['stock'] . '" name="stock['. $row_index .']">' . $rows['stock'] . '</td>';
				echo '<td><input type="text" value="" name="quantity['. $row_index .']" tabindex="' . $tab_index++ .'" /></td>';
				echo '<td><select name="delivery['. $row_index .']" tabindex="' . $tab_index++ .'">
						<option value="Any">Any</option>
						<option value="Emergency">Emergency</option>
						<option value="Next Day">Next Day</option>
						<option value="2-3 Days">2-3 Days</option>
						<option value="4-6 Days">4-6 Days</option>
						<option value="7-10 Days">7-10 Days</option>
						<option value="10+ Days">10+ Days</option>
					</select></td>';
				echo '</tr>';

				// Iterate for next row
				$row_index++;
			}

			echo '</table>';

			// Send request for quotation
			echo '<div id="submit-rfq">';
			echo '<input type="submit" name ="send-rfq" value="Request Quotation">';
			echo '<p><a href="#new-search">Back to top</a></p>';
			echo '</div>';
			echo '</form>';

			echo '</div>';
		}

	// Free result
	mysqli_free_result($result);
	}

// Close Connection
mysqli_close($link);

}

Sorry for so much code - i realise its really explicit and not very ocncise!

[bradgrafelman]

One thing I notice is that you're trying to do a lot of blacklisting, e.g. checking to make sure a list of regular expressions don't appear in a given input. In most situations, this is either virtually or truly impossible or improbable. Instead, it's often a lot easier/better to do whitelisting, e.g. specify a pattern that the input should be versus everything it shouldn't be.

[Major_Disaster]

Thank you for all your help bradgrafelman - its much appreciated.

I will look into moving to white listing, but failing any glaring security hole on my code i will mark this thread as resolved (though i do have another issue now though that i will post and would really love some help with!)

Thanks again