PEAK XOOPS - News 
The DBLayer Trapping anti-SQL-Injection of Protector-3.30 with XOOPS2 raises a "SQL Injection" alert at updating preferences including doublequatation(").
This wrong detection is caused by the wrong way to escape SQL.
class/database/mysqldatabase.php
function quoteString($str)
{
$str = "'".str_replace('\\"', '"', addslashes($str))."'";
return $str;
}
Only XOOPS2 and XCL2.1 have such a wrong escaping method.
This method should be corrected like:
function quoteString($str)
{
$str = "'".str_replace('\\"', '"', addslashes($str))."'";
$str = "'".mysql_real_escape_string($str)."'";
return $str;
}
On the other hand, both ImpressCMS and XOOPS-2.3.2 have a right method.
However this is just a problem of the XOOPS Cube project, there can be some modules/hacks escaping SQLs like this.
Then I have to modify the logic of the "DBLayer Trapping anti-SQL-Injection".
(A) A request including ' or " is found in a SQL as is (without escaping)
(B) All body of the request stays in single string of the SQL (not breaking quotation)
When both (A) AND !(B) are found, protector stops the program as "SQL Injection found".
This logic can reduce some patterns of wrong detections.
We have to know comparing doubtful requests and all SQLs in the DB Layer have to pay not a few CPU band.
Then, we can override the DB Layer only when "attackable requests" has come.
Almost all HTTP requests never have such "attackable requests".
This logic make it compatible the speed and the security.
My protector find "attackable requests" by this pattern (perl regex formatted):
/(information_schema|select|'|")/i
' and " can break the pair of quatations.
select allows attackers to access the other tables.
On the other hand, it ignores union because it has non-sense without select.
Marks starting commatation like (/* , -- or #) are ignored too.
Such marks can get their meaning only when the pair of quatitions are broken.
And $_SERVER['HTTP_ACCEPT'] often incldues a string like '*/*'.
Let's go to the logic of the method query() of the Protector's DB Layer.
query() compares "attackable requests" and all SQLs.
There are two patterns of vulnerabilities against SQL Injection.
(1) a string missing to escape (the string is origined from a request)
eg)
SELECT ... FROM `table` WHERE `varchar_column`='(string_missing_to_escape)'
(2) a request placed into SQL as is
eg)
SELECT ... FROM `table` WHERE `integer_column`=(request)
SELECT ... FROM `table` WHERE ... ORDER BY (request)
This logic can protect almost all vulnerabilities like (1).
- list requests having ' or " up
- compare all SQLs and the listed requests
- if a SQL includes one of the listed requests, stop it.
Because ' or " should be escaped in all SQL.
Of course, this logic can be too sensitive.
To avoid accidental matches, we have to set the minimum length of "attackable requests".
I guess "6" is the best value for the length.
Because the shortest "attackable request" is here.
'OR 1#
On the other hand, we cannot cover the mistakes like (2) entirely.
The data in the `table` can be read/lost/added as attackers like.
But we can protect the other tables by these logics even if a vulnerablity like (2) exists.
(A) SQL with any comment
(B) If "requests including SELECT" exists outside of quotation.
SQL both !(A) and !(B) can be queried.
Though (A) looks too sensible, it is not troublesome at my test.
Note, this overridding is rarely occured.
(B) is the main logic.
All attacks aiming some specific tables have to include "SELECT".
To know the list of tables, attackers have to use "SELECT" from information_schema.
If you want to know the implementation, try to read ProtectorMySQLDatabase.php in Protector-3.3
To Compare request and SQL, we have to override DB layer.
With XOOPS, this will be implemented as a modification for databasefactory.php because the database factory class looks too rigid.
This is my modification.
It might be not the best way, but better way for adopted by each core teams of XOOPS forks/folks.
class/database/databasefactory.php
require_once $file;
/* patch from */
if ( defined('XOOPS_DB_ALTERNATIVE') && class_exists( XOOPS_DB_ALTERNATIVE ) ) {
$class = XOOPS_DB_ALTERNATIVE ;
} else /* patch to */if (!defined('XOOPS_DB_PROXY')) {
$class = 'Xoops'.ucfirst(XOOPS_DB_TYPE).'DatabaseSafe';
} else {
$class = 'Xoops'.ucfirst(XOOPS_DB_TYPE).'DatabaseProxy';
}
$instance =& new $class();
hi minahito, marcan, and phppp.
I've made the patch can be accepted for you.
Please consider it.
At the next article, I will discuss about the condition when the db layer must be overridden, and the logic comparing requests and SQL.
I've shocked just by looking inside of the archive of xoops-2.3.2b.
They put XOOPS_TRUST_PATH folder inside htdocs/ !
(They renamed xoops_trust_path into xoops_lib. this fact also shows us they didnot understand the meaning of XOOPS_TRUST_PATH)
Moreover, there are no .htaccess under the folder xoops_lib/
I suspect my eyes.
mamba had reported LFI in the file under XOOPS_TRUST_PATH.
This is another evidence they cannot understand the meaning of inside/outside DocumentRoot.
When mamba said "I fixes Protector", I replied "Such a patch is non-sense".
This report proves mamba's patch was just non-sense.
http://www.milw0rm.com/exploits/7705
You should interpret the report is not an exploit of Protector itself but just XOOPS-2.3.2.
Anyway, phppp and developpers of xoops.org should do right now:
Put xoops_lib(XOOPS_TRUST_PATH) ouside of htdocs.
Learn the meanining of inside/outside DocumentRoot.
Read how to install Protector V3 again and again!
If you cannot do that or cannot understand what I mean, remove Protector from your archive.
Your wrong structure of the archive gave me pain.
My module -Protector- is useful for protecting all XOOPS forks/folks from maricious attacks as long as the module is installed rightly.
You know D3 modules can be installed any dirnames as you like.
The exact "dirname rule" by perl regex:
[0-9a-zA-Z_-]{,25}
Under the environtment "case sensitive" (Unix etc.), (dirname) is distinguished (Dirname) definitely.
eg) XUGJ uses d3forum as "QandA".
http://www.xugj.org/modules/QandA/ OK
http://www.xugj.org/modules/qanda/ NG
And I believed both (dirname) and (Dirname) can be installed independently with such environtments.
To our regret, this is just a misunderstood.
Consider XOOPS with "pico".
I dare to install another pico as "PICO".
This SQL is queried to identify "dirname".
SELECT (snip) WHERE dirname='PICO'
'PICO' will be hit existent 'pico'.
And the installer raise an error.
Of course, this is not only a problem of installer but common.php also.
If you want both "pico" and "PICO", try this query once.
ALTER TABLE (prefix)_modules MODIFY `dirname` varchar(25) binary NOT NULL default '';
The column of `dirname` is added an attribute 'binary'.
I've confirmed "pico" can work fine with "PICO".
But only the feature of "Literal overridding" cannot work independently between "pico" and "PICO".
This code is the cause of the confusion.
$constpref = '_MI_' . strtoupper( $mydirname ) ;
We -D3 module developpers- have to fix this code in (lang)/modinfo.php, (lang)/block_each.php and xoops_version.php etc.