[Resolvido] Prepared statements vunerável a ataques

[Th3Alchemist 1]

Boas!

Fiz o seguinte código, utilizando a extensão mysqli_*, assim como prepared statements, porém segundo o sqlmap, isto continua vunerável:

$id        = $_GET['id'];



//Connect to MySQL database:
$db       = new mysqli(DB_HOST, DB_USER, DB_PASSWORD, DB_DATABASE);
if ($db->connect_errno) {
   echo "Failed to connect to MySQL: (" . $db->connect_errno . ") " . $db->connect_error;
}
//Define charset:
if (!$db->set_charset("utf8")) {
   echo "Failed to set charset: " . $db->error;
}
$id = $db->real_escape_string($id);
//Prepare statement:
if(!($stmt = $db->prepare("SELECT post.PO_title, post.PO_abstract, post.PO_content, post.PO_datetime FROM post WHERE post.PO_ID = {$id}"))) {
   echo "Prepare failed: (" . $db->errno . ") " . $db->error;
}
//Bind results:
if(!($stmt->bind_result($title, $abstract, $content, $datetime))) {
   echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
}
//Execute query:
if(!($stmt->execute())) {
   echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
}

$stmt->fetch();

mysqli_close($db);

Como podem ver, o $id é recebido por um parâmetro $_GET

Uma forma de eu evitar a injeção SQL é simplesmente adicionando:

$id        = (INT)$_GET['id'];

Porém o sqlmap detecta isso... O que me falta?

[HappyHippyHippo 1,162]

não é essa a maneira de usar prepared statements

já estás a dar um SQL construído onde deverias dar a ser constrito pela classe

$stmt = $db->prepare("SELECT post.PO_title, post.PO_abstract, post.PO_content, post.PO_datetime FROM post WHERE post.PO_ID = ?"))
$stmt->bind_param("i", $_GET['id']); // dizer para usar o valor $_GET['id'] do tipo inteiro no lugar do ?

[mjamado 42]

O problema é que não estás a usar prepared statements.

$id = $db->real_escape_string($id);
//Prepare statement:
if(!($stmt = $db->prepare("SELECT post.PO_title, post.PO_abstract, post.PO_content, post.PO_datetime FROM post WHERE post.PO_ID = {$id}"))) {
echo "Prepare failed: (" . $db->errno . ") " . $db->error;
}

Isto é concatenação directa da query e está oh so wrong. É assim que tens de fazer:

//Prepare statement:
if(!($stmt = $db->prepare("SELECT post.PO_title, post.PO_abstract, post.PO_content, post.PO_datetime FROM post WHERE post.PO_ID = :id"))) {
echo "Prepare failed: (" . $db->errno . ") " . $db->error;
}
$stmt->bindParam(":id", $id, PDO::PARAM_INT);

Repara que tirei aquela coisa do escape, que não está lá a fazer nada.

[Th3Alchemist 1]

Pois realmente não utilizei muito a cabeça.

Obrigado!

E já agora, uma vez definindo bem as prepared statements, é suficiente contra injecções?

Edit:

Adicionei o código fornecido pelo HappyHippyHippo, mas continua vunerável :S

Única diferença é que o ataque demora muito mais

$id = $_GET['id'];

//Connect to MySQL database:
$db       = new mysqli(DB_HOST, DB_USER, DB_PASSWORD, DB_DATABASE);
if ($db->connect_errno) {
   echo "Failed to connect to MySQL: (" . $db->connect_errno . ") " . $db->connect_error;
}

//Define charset:
if (!$db->set_charset("utf8")) {
   echo "Failed to set charset: " . $db->error;
}

//Prepare statement:
if(!($stmt = $db->prepare("SELECT post.PO_title, post.PO_abstract, post.PO_content, post.PO_datetime FROM post WHERE post.PO_ID = ?"))) {
   echo "Prepare failed: (" . $db->errno . ") " . $db->error;
}

if(!($stmt->bind_param("i", $id))) {
   echo "Failed to bind parameters: (" . $stmt->errno . ") " . $stmt->error;
}

//Bind results:
if(!($stmt->bind_result($title, $abstract, $content, $datetime))) {
   echo "Binding results failed: (" . $stmt->errno . ") " . $stmt->error;
}
//Execute query:
if(!($stmt->execute())) {
   echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
}

$stmt->fetch();

mysqli_close($db);

Edited March 5, 2013 by Th3Alchemist

[brunoais 65]

Em que situação é que ele é vulnerável? Qual é o input que é usado para considerar vulnerável?

[Th3Alchemist 1]

Log do sqlmap:

marcsoler@R510:~/Transferências/sqlmapproject-sqlmap-9e49d8c$ python sqlmap.py -u "http://localhost/blog/index.php?page=post&id=1" --dbs

sqlmap/1.0-dev - automatic SQL injection and database takeover tool
http://sqlmap.org

[!] legal disclaimer: Usage of sqlmap for attacking targets without prior mutual consent is illegal. It is the end user's responsibility to obey all applicable local, state and federal laws. Developers assume no liability and are not responsible for any misuse or damage caused by this program

[*] starting at 09:37:23

[09:37:23] [iNFO] testing connection to the target url
[09:37:23] [iNFO] heuristics detected web page charset 'ISO-8859-2'
[09:37:23] [iNFO] testing if the url is stable, wait a few seconds
[09:37:24] [WARNING] url is not stable, sqlmap will base the page comparison on a sequence matcher. If no dynamic nor injectable parameters are detected, or in case of junk results, refer to user's manual paragraph 'Page comparison' and provide a string or regular expression to match on
how do you want to proceed? [©ontinue/(s)tring/®egex/(q)uit]
[09:37:26] [iNFO] testing if GET parameter 'page' is dynamic
[09:37:26] [iNFO] heuristics detected web page charset 'utf-8'
[09:37:26] [iNFO] confirming that GET parameter 'page' is dynamic
[09:37:26] [iNFO] GET parameter 'page' is dynamic
[09:37:26] [WARNING] reflective value(s) found and filtering out
[09:37:26] [WARNING] heuristic (parsing) test shows that GET parameter 'page' might not be injectable
[09:37:26] [iNFO] testing for SQL injection on GET parameter 'page'
[09:37:26] [iNFO] testing 'AND boolean-based blind - WHERE or HAVING clause'
[09:37:26] [iNFO] testing 'MySQL >= 5.0 AND error-based - WHERE or HAVING clause'
[09:37:26] [iNFO] testing 'PostgreSQL AND error-based - WHERE or HAVING clause'
[09:37:27] [iNFO] testing 'Microsoft SQL Server/Sybase AND error-based - WHERE or HAVING clause'
[09:37:27] [iNFO] testing 'Oracle AND error-based - WHERE or HAVING clause (XMLType)'
[09:37:27] [iNFO] testing 'MySQL inline queries'
[09:37:27] [iNFO] testing 'PostgreSQL inline queries'
[09:37:27] [iNFO] testing 'Microsoft SQL Server/Sybase inline queries'
[09:37:27] [iNFO] testing 'Oracle inline queries'
[09:37:27] [iNFO] testing 'SQLite inline queries'
[09:37:27] [iNFO] testing 'MySQL > 5.0.11 stacked queries'
[09:37:27] [iNFO] testing 'PostgreSQL > 8.1 stacked queries'
[09:37:27] [iNFO] testing 'Microsoft SQL Server/Sybase stacked queries'
[09:37:27] [iNFO] testing 'MySQL > 5.0.11 AND time-based blind'
[09:37:28] [iNFO] testing 'PostgreSQL > 8.1 AND time-based blind'
[09:37:28] [iNFO] testing 'Microsoft SQL Server/Sybase time-based blind'
[09:37:28] [iNFO] testing 'Oracle AND time-based blind'
[09:37:28] [iNFO] testing 'MySQL UNION query (NULL) - 1 to 10 columns'
[09:37:30] [iNFO] testing 'Generic UNION query (NULL) - 1 to 10 columns'
[09:37:30] [WARNING] using unescaped version of the test because of zero knowledge of the back-end DBMS. You can try to explicitly set it using option '--dbms'
[09:37:31] [WARNING] GET parameter 'page' is not injectable
[09:37:31] [iNFO] testing if GET parameter 'id' is dynamic
[09:37:31] [iNFO] confirming that GET parameter 'id' is dynamic
[09:37:31] [iNFO] GET parameter 'id' is dynamic
[09:37:32] [iNFO] heuristic (parsing) test shows that GET parameter 'id' might be injectable (possible DBMS: 'PostgreSQL')
[09:37:32] [iNFO] testing for SQL injection on GET parameter 'id'
heuristic (parsing) test showed that the back-end DBMS could be 'PostgreSQL'. Do you want to skip test payloads specific for other DBMSes? [Y/n] y
do you want to include all tests for 'PostgreSQL' ignoring provided level (1) and risk (1)? [Y/n] y
[09:37:45] [iNFO] testing 'AND boolean-based blind - WHERE or HAVING clause'
[09:37:46] [iNFO] GET parameter 'id' is 'AND boolean-based blind - WHERE or HAVING clause' injectable
[09:37:46] [iNFO] testing 'PostgreSQL AND error-based - WHERE or HAVING clause'
[09:37:47] [iNFO] testing 'PostgreSQL OR error-based - WHERE or HAVING clause'
[09:37:47] [iNFO] testing 'PostgreSQL error-based - Parameter replace'
[09:37:47] [iNFO] testing 'PostgreSQL inline queries'
[09:37:47] [iNFO] testing 'PostgreSQL > 8.1 stacked queries'
[09:37:47] [iNFO] testing 'PostgreSQL stacked queries (heavy query)'
[09:37:47] [iNFO] testing 'PostgreSQL < 8.2 stacked queries (Glibc)'
[09:37:47] [iNFO] testing 'PostgreSQL > 8.1 AND time-based blind'
[09:37:48] [iNFO] testing 'PostgreSQL > 8.1 AND time-based blind (comment)'
[09:37:48] [iNFO] testing 'PostgreSQL AND time-based blind (heavy query)'
[09:37:48] [iNFO] testing 'PostgreSQL AND time-based blind (heavy query - comment)'
[09:37:48] [iNFO] testing 'PostgreSQL > 8.1 OR time-based blind'
[09:37:48] [iNFO] testing 'PostgreSQL OR time-based blind (heavy query)'
[09:37:48] [iNFO] testing 'PostgreSQL > 8.1 time-based blind - Parameter replace'
[09:37:48] [iNFO] testing 'PostgreSQL time-based blind - Parameter replace (heavy query)'
[09:37:49] [iNFO] testing 'Generic UNION query (NULL) - 1 to 20 columns'
[09:37:49] [iNFO] automatically extending ranges for UNION query injection technique tests as there is at least one other potential injection technique found
[09:37:49] [iNFO] ORDER BY technique seems to be usable. This should reduce the time needed to find the right number of query columns. Automatically extending the range for current UNION query injection technique test
[09:37:50] [iNFO] target url appears to have 1 column in query
[09:37:50] [WARNING] if UNION based SQL injection is not detected, please consider and/or try to force the back-end DBMS (e.g. --dbms=mysql)
[09:37:50] [iNFO] checking if the injection point on GET parameter 'id' is a false positive
GET parameter 'id' is vulnerable. Do you want to keep testing the others (if any)? [y/N] n
sqlmap identified the following injection points with a total of 221 HTTP(s) requests:
---
Place: GET
Parameter: id
Type: boolean-based blind
Title: AND boolean-based blind - WHERE or HAVING clause
Payload: page=post&id=1 AND 2701=2701
---
[09:37:56] [iNFO] testing PostgreSQL
[09:37:57] [WARNING] the back-end DBMS is not PostgreSQL
[09:37:57] [iNFO] testing MySQL
[09:37:57] [iNFO] confirming MySQL
[09:37:58] [iNFO] the back-end DBMS is MySQL
web application technology: Apache 2.4.3, PHP 5.4.7
back-end DBMS: MySQL >= 5.0.0
[09:37:58] [iNFO] fetching database names
[09:37:58] [iNFO] fetching number of databases
[09:37:58] [WARNING] running in a single-thread mode. Please consider usage of option '--threads' for faster data retrieval
[09:37:58] [iNFO] retrieved: 3
[09:37:59] [iNFO] retrieved: information_schema
[09:38:34] [iNFO] retrieved: marcsoler
[09:38:54] [iNFO] retrieved: test
available databases [3]:
[*] information_schema
[*] marcsoler
[*] test

[09:39:03] [iNFO] fetched data logged to text files under '/home/marcsoler/Transferências/sqlmapproject-sqlmap-9e49d8c/output/localhost'

[*] shutting down at 09:39:03

Como podem, o parâmetro GET id continua injetável, que por sua vez utiliza prepared statements

[mjamado 42]

Uma ferramenta automática nunca há-de substituir uns bons miolos.

Ora pensa lá o que é que a conversão para inteiro de "1 AND 2701=2701" dá, e que diferenças é que o resultado terá em relação a apenas "1".

[Th3Alchemist 1]

Uma ferramenta automática nunca há-de substituir uns bons miolos.

Ora pensa lá o que é que a conversão para inteiro de "1 AND 2701=2701" dá, e que diferenças é que o resultado terá em relação a apenas "1".

terá a mesma como somente "1", mas AND e = como são string como são aceites ao fazer prepare?

Edited March 6, 2013 by Th3Alchemist

[HappyHippyHippo 1,162]

page=post&id=1 AND 2701=2701

isto dá SQL Injection ? estará tudo que aprendi até agora errado ?

[mjamado 42]

terá a mesma como somente "1", mas AND e = como são string como são aceites ao fazer prepare?

Porque o PHP é uma linguagem de tipagem dinâmica. Há um cast implícito, que faz com que a query seja igual. Logo, não há SQL injection. A ferramenta é que é burra.

[Th3Alchemist 1]

A ferramenta é que é burra.

Tens razão limpei a cache da ferramenta e parece ser não injetável agora.

[12:19:52] [WARNING] GET parameter 'id' is not injectable
[12:19:52] [CRITICAL] all tested parameters appear to be not injectable. Try to increase '--level'/'--risk' values to perform more tests. Also, you can try to rerun by providing either a valid value for option '--string' (or '--regexp')

Tópico marcado como resolvido. Obrigado a todos!

Edited March 6, 2013 by Th3Alchemist