MantisBT

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005777AContentContentpublic2017-03-24 19:582017-04-07 20:20
Reportergreg 
Assigned Tocindy 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version 
Target Version1.4Fixed in Version 
Summary0005777: $addslashes broken
DescriptionWith the switch toward mysqli, the old mysql_real_escape_string is deprecated and soon to disappear. Because mysqli_real_escape_string and $db->real_escape_string require a DB connection, $addslashes no longer works to escape quotes when text is sent to the DB
Additional InformationThe last commit, where the problem exists.
https://github.com/atutor/AContent/commit/86ba9a3db0bc9cf97e3697eeefa75b62ad216aa2 [^]

$addslashes is used 492 time in AContent, so hoping there a way to fix $addslashes, instead of having to replace them all.
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0007559)
greg (administrator)
2017-03-24 20:05

using mysql_real_escape_string works for now, but will fail in PHP 7, and it generates a lot of deprecated messages in the log.
(0007560)
cindy (administrator)
2017-04-06 20:55

As we've known, $addslashes used in AContent is an equivalant of mysql_real_escape_string() that is to escape the string data being saved into the mysql databases to prevent the SQL injection.

After doing some research on how to prevent the SQL injection with mysqli, see:
http://stackoverflow.com/questions/2214244/htmlentities-vs-addslashes-vs-mysqli-real-escape-string [^]
https://www.youtube.com/watch?v=etPK7BBp-eQ [^]

the recommended way is to use mysqli_stmt::prepare/mysqli_stmt_prepare to prepare the SQL statement instead of using mysqli_real_escape_string. See http://php.net/manual/en/mysqli-stmt.prepare.php. [^]

In my opinion, the proper way to fix this issue is:
1. Abandon the use of $addslashes;
2. Improve the execute() function in DAO.class.php to use mysqli_stmt::prepare/mysqli_stmt_prepare work flow to guard and execute SQL statements using mysqli type databases, and continue to use mysql_real_escape_string() to guard statements for mysql databases. DAO.class.php is the base SQL statement handler inherited by all other DAO classes. Enhancing it would automatically enhance all DAO classes.

I've searched AContent source code for all spots that $addslashes is used. Most usages are for guarding SQL statements but there are a few locations that $addslashes is misused for verifying form data. Those locations should be carefully re-evaluated to switch to use htmlentities() or the actual addslashes(). Once those locations are fixed, $addslashes can be completed abandoned.

I know this is not a quick fix. It would affect many files. The benefit is it will produce a more secured AContent.

According to the PHP announcement for mysql: mysql extension is deprecated as of PHP 5.5.0, and has been removed as of PHP 7.0.0. See http://php.net/manual/en/intro.mysql.php. [^] Probably it's time to consider to only support:
1. mysqli;
2. PHP 5.5.0 and above
(0007562)
greg (administrator)
2017-04-07 20:20

Thanks Cindy. I was afraid you would say that. Looks there some work to do to fix this.

- Issue History
Date Modified Username Field Change
2017-03-24 19:58 greg New Issue
2017-03-24 19:58 greg Status new => assigned
2017-03-24 19:58 greg Assigned To => cindy
2017-03-24 20:05 greg Note Added: 0007559
2017-04-06 20:55 cindy Note Added: 0007560
2017-04-07 20:20 greg Note Added: 0007562


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker