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.
There are a lot of XOOPS modules or PHP applications with SQL Injection still.
Protector can protect some patterns of SQL Injections.
It is just "some" instead of "all".
That's because Protector cannot distinguish attacks and fair requests just by REQUEST layer.
A word of "UNION" will be posted as "UNI-ON".
Such modification in REQUEST layer must be non-sense.
Then, Protector should judge them by both REQUEST layer and DB layer.
anti-XSS:
(a) doubtful requests are found
(b) ob_start()
(c) compare requests and outputs
SQL Injection:
(1) doubtful requests are found
(2) override DB layer
(3) compare requests and SQLs
It is not easy to implement (3) because Protector have to parse SQLs.
However, (2) will be the most problem for XOOPS.
Because there are no way to override DB layer.
I will suggest DB layer modification can be overridden for all core teams in the next entry.
This idea is based on JM2. (four years ago!)
He is the real hero for XOOPS 