0161 513 0960Jack O'Sullivan
March 22 2021
This will be the first in an occasional series giving the technical details of interesting vulnerabilities we've discovered over the years. So I had thought this might have been the last SQLi in Magento, then I remembered this awesome vulnerability/exploit from checkpoint http://blog.checkpoint.com/2015/04/20/analyzing-magento-vulnerability/. We're not looking at anything nearly as complicated as that :)
A colleague spotted unusual behaviour in a particular endpoint in the app he was testing in a black box test:
- Whenever "%" was used in two specific parameters it caused an error
- Whenever "%%" was used the app didn't error
As it happened I'd recently been doing some stuff with PHP format strings and recognised that our input was probably being processed as part of a format string. Part way through looking at the issue we established that the app was Magento and we could look at the source code, in all honesty I can't remember if we exploited it blind or with the source code, but it's far easier to understand the issue by looking at the source code:
|
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34 |
/**
* Add filter by attribute rated price
*
* @param Mage_CatalogSearch_Model_Resource_Advanced_Collection $collection
* @param Mage_Catalog_Model_Resource_Eav_Attribute $attribute
* @param string|array $value
* @param int $rate
* @return bool
*/
public function addRatedPriceFilter($collection, $attribute, $value, $rate = 1)
{
$adapter = $this->_getReadAdapter();
$conditions = array();
if (strlen($value['from']) > 0) {
$conditions[] = $adapter->quoteInto('price_index.min_price %s * %s >= ?', $value['from']);
}
if (strlen($value['to']) > 0) {
$conditions[] = $adapter->quoteInto('price_index.min_price %s * %s <= ?', $value['to']);
}
if (!$conditions) {
return false;
}
$collection->addPriceData();
$select = $collection->getSelect();
$response = $this->_dispatchPreparePriceEvent($select);
$additional = join('', $response->getAdditionalCalculations());
foreach ($conditions as $condition) {
$select->where(sprintf($condition, $additional, $rate));
}
return true;
}
|
The above function is called with user controlled values in $value. Can you see the problem? - the quoteInto function (which is intended to prevent SQL injection) is run before the output is processed through sprintf function. Any sanitisation function should always be run immediately before the value is used, otherwise any intervening functions might be abused to subvert the value. In this case we need to find a way to abuse the fact that the value is processed through sprintf.
Since $value['from'] and $value['to'] are processed in exactly the same way we'll focus on a single parameter $value['from'], so we end up with "%s * %s <= '#OURVALUE#'" being processed through sprintf where #OURVALUE# has been escaped but is otherwise completely under our control. There is no use our trying to add references to variables of the form %s, %d or anything like that as these variables simply won't be supplied. Fortunately we can refer to any supplied variable using the position specifier notation, so for instance %$2s will format the second argument as a string.
PHP - How do I love thee, let me count the ways
PHP has a lovely habit (from a hackers point of view) of attempting to process or ignore invalid input. If we ask sprintf to format a variable with an unknown format, it decides the best thing to do is output nothing. How does this help us? Let's see what happens if we format a variable with the "\" formatting (which doesn't exist).....
So if we place "$1'" into our input, when escaped it will become "$1\'" which sprintf will view as variable formatted as "\" followed by a single apostrophe..... PHP decides to ignore the $1\ since "\" is not a valid format and leaves behind only the ' - woohoo SQL injection achieved.
Unfortunately the generated SQL is used in multiple queries before user output is produced, so it's not possible to do a simple UNION to extract data. However as long as the magento instance contains at least one product for sale, we can carry out simple boolean queries by accessing a URL of the form:
index.php/catalogsearch/advanced/result/?name=&description=&short_description=&sku=&price[from]=0&price[to]=0%1$%27)+or+1=1--+&search=Search
| The output for a false condition |
| The output for a true condition |
You can download the affected version (1.6.0.0) from https://magento.com/tech-resources/download under RELEASE ARCHIVE.
