php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #77531 MySQLi example code promotes bad security practice
Submitted: 2019-01-27 22:48 UTC Modified: 2020-12-26 18:12 UTC
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:0 of 1 (0.0%)
From: marcus dot watson at loumiaconsulting dot com Assigned: dharman (profile)
Status: Closed Package: MySQLi related
PHP Version: Irrelevant OS:
Private report: No CVE-ID: None
 [2019-01-27 22:48 UTC] marcus dot watson at loumiaconsulting dot com
Description:
------------
---
From manual page: https://php.net/mysqli.examples-basic
---

If this is to be a basic example of database handling and you do not want to use parameterization or promote security for the sake of brevity, I suggest using an approach that does not include string concatenation.

The current example is a bad habit to promote to developers. Developers unaware of SQL injection will focus on the "$result = $mysqli->query($sql)" part and omit any validation that has been performed previously. Typical InfoSec best practice dictates implementing parameterization first, *then* validation.


Suggested alternative: Use a hard coded query to return a status (eg total number of actors/movies in the database, or who is the customer with the largest number of fines).

Leave the dynamic queries for another example altogether, where the correct approach can be demonstrated, and cross-referenced from this page if dynamic queries are required.

This approach would demonstrate static queries with single/multiple rows. The more complex example would include dynamic queries with single/multiple rows, thus covering the main scenarios that teams would encounter.


I'm happy to collaborate with the assignee to formulate the code.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-01-28 12:50 UTC] girgias@php.net
-Summary: MySQLi example code promotes security bad practice +Summary: MySQLi example code promotes bad security practice -Package: Documentation problem +Package: MySQLi related
 [2019-01-28 12:50 UTC] girgias@php.net
Hello Marcus,

First of all, thank you for pointing this out and be willing to collaborate.

The best way to proceed would be for you to edit the documentation with the help of the online doc editor located at https://edit.php.net (or use the direct link from the manual to edit this specific page https://edit.php.net/?project=PHP&perm=en/mysqli.examples-basic.php)

After having edited the corresponding XML file, submit a patch via the editor and optionally submit the patch to this bug report.
So that a member of the doc team can review it and accept it.

Best regards.
 [2020-12-16 15:31 UTC] craig at craigfrancis dot co dot uk
Sorry, I seem to have created a duplicate bug report at:

https://bugs.php.net/bug.php?id=80520

Which includes details on two PRs on the GitHub repo.
 [2020-12-26 18:12 UTC] dharman@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: dharman
 [2020-12-26 18:12 UTC] dharman@php.net
Thank you for bringing this to our attention. In the end, we have decided to remove this page completely. The example did not show the best security practices as you said (prepared statements, proper output escaping, proper error handling, etc.). This has been brought to our attention a number of times, but fixing this would require someone actually writing a full mysqli tutorial, which nobody has done so far. There are examples of both static query and prepared statement execution in the mysqli quick book which should suffice. 

We appreciate your help in helping to maintain mysqli/PHP documentation. If you would like to contribute more, you are welcome to make GitHub PRs at https://github.com/php/doc-en
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Thu Jan 28 06:01:26 2021 UTC