I'm currently working on an already modified version of magento (v 1.6.1). The previous developers have modified the app/core itself, what if I upgrade to the 1.7? It would restore original app/core, am I right? (because I know every mod should be placed under app/local)
Then I noticed, by running diff on the ecommerce and a clean 1.6.1 installation that the developers have applied this modification (lines marked with "<" was the original content and ">" the edited one)
diff app/code/core/Mage/Checkout/controllers/CartController.php
169c169,170
< $params['qty'] = $filter->filter($params['qty']);
---
> #$params['qty'] = $filter->filter($params['qty']);
> $params['qty'] = $params['qty'];
311c312,313
< $params['qty'] = $filter->filter($params['qty']);
---
> #$params['qty'] = $filter->filter($params['qty']);
> $params['qty'] = $params['qty'];
383c385,386
< $cartData[$index]['qty'] = $filter->filter(trim($data['qty']));
---
> //$cartData[$index]['qty'] = $filter->filter(trim($data['qty']));
> $cartData[$index]['qty'] = $data['qty'];
As you may notice they disabled $filter->filter and trim.
Doesn't this expose the e-store to SQLInjections or similiar arbitrary code execution? Is there another check that magento performs before to store this data inside the database?
The filter functions that the previous developers removed are not used to filter input for SQL injection or other security risks. They are used for converting localized input to a standard form that can be processed regardless of locale. Here's the expanded context for the first diff:
$filter = new Zend_Filter_LocalizedToNormalized(
array('locale' => Mage::app()->getLocale()->getLocaleCode())
);
$params['qty'] = $filter->filter($params['qty']);
See the Zend documentation for details on what LocalizedToNormal does.
Magento has built-in safeguards to prevent SQL injection by using standard database classes that filter all data before constructing a query. That logic is located in the Mage_Core_Model_Resource_* classes as well as the Zend libraries stored in /lib/Zend. As long as the previous developers didn't modify those classes, there shouldn't be additional SQL risk.
Cross-site scripting is always a potential issue, of course, but the risk there typically lies more at the View layer (PHTML & Block classes) than at the Controller or Model layers.