1

I'm modifying the search module of prestashop in order to be able to search strings containing dots : .
Prestashop uses its own sanitize functions against mysql injection vulnerability (yeah...) and uses a list of characters (almost entierly copied from Drupal search module) to filter:

define('PREG_CLASS_SEARCH_EXCLUDE',
'\x{0}-\x{2c}\x{2e}-\x{2f}\x{3a}-\x{40}\x{5b}-\x{60}\x{7b}-\x{bf}\x{d7}\x{f7}\x{2b0}-'.
'\x{385}\x{387}\x{3f6}\x{482}-\x{489}\x{559}-\x{55f}\x{589}-\x{5c7}\x{5f3}-'.
'\x{61f}\x{640}\x{64b}-\x{65e}\x{66a}-\x{66d}\x{670}\x{6d4}\x{6d6}-\x{6ed}'.
'\x{6fd}\x{6fe}\x{700}-\x{70f}\x{711}\x{730}-\x{74a}\x{7a6}-\x{7b0}\x{901}-'.
'\x{903}\x{93c}\x{93e}-\x{94d}\x{951}-\x{954}\x{962}-\x{965}\x{970}\x{981}-'.
'\x{983}\x{9bc}\x{9be}-\x{9cd}\x{9d7}\x{9e2}\x{9e3}\x{9f2}-\x{a03}\x{a3c}-'.
'\x{a4d}\x{a70}\x{a71}\x{a81}-\x{a83}\x{abc}\x{abe}-\x{acd}\x{ae2}\x{ae3}'.
'\x{af1}-\x{b03}\x{b3c}\x{b3e}-\x{b57}\x{b70}\x{b82}\x{bbe}-\x{bd7}\x{bf0}-'.
'\x{c03}\x{c3e}-\x{c56}\x{c82}\x{c83}\x{cbc}\x{cbe}-\x{cd6}\x{d02}\x{d03}'.
'\x{d3e}-\x{d57}\x{d82}\x{d83}\x{dca}-\x{df4}\x{e31}\x{e34}-\x{e3f}\x{e46}-'.
'\x{e4f}\x{e5a}\x{e5b}\x{eb1}\x{eb4}-\x{ebc}\x{ec6}-\x{ecd}\x{f01}-\x{f1f}'.
'\x{f2a}-\x{f3f}\x{f71}-\x{f87}\x{f90}-\x{fd1}\x{102c}-\x{1039}\x{104a}-'.
'\x{104f}\x{1056}-\x{1059}\x{10fb}\x{10fc}\x{135f}-\x{137c}\x{1390}-\x{1399}'.
'\x{166d}\x{166e}\x{1680}\x{169b}\x{169c}\x{16eb}-\x{16f0}\x{1712}-\x{1714}'.
'\x{1732}-\x{1736}\x{1752}\x{1753}\x{1772}\x{1773}\x{17b4}-\x{17db}\x{17dd}'.
'\x{17f0}-\x{180e}\x{1843}\x{18a9}\x{1920}-\x{1945}\x{19b0}-\x{19c0}\x{19c8}'.
'\x{19c9}\x{19de}-\x{19ff}\x{1a17}-\x{1a1f}\x{1d2c}-\x{1d61}\x{1d78}\x{1d9b}-'.
'\x{1dc3}\x{1fbd}\x{1fbf}-\x{1fc1}\x{1fcd}-\x{1fcf}\x{1fdd}-\x{1fdf}\x{1fed}-'.
'\x{1fef}\x{1ffd}-\x{2070}\x{2074}-\x{207e}\x{2080}-\x{2101}\x{2103}-\x{2106}'.
'\x{2108}\x{2109}\x{2114}\x{2116}-\x{2118}\x{211e}-\x{2123}\x{2125}\x{2127}'.
'\x{2129}\x{212e}\x{2132}\x{213a}\x{213b}\x{2140}-\x{2144}\x{214a}-\x{2b13}'.
'\x{2ce5}-\x{2cff}\x{2d6f}\x{2e00}-\x{3005}\x{3007}-\x{303b}\x{303d}-\x{303f}'.
'\x{3099}-\x{309e}\x{30a0}\x{30fb}\x{30fd}\x{30fe}\x{3190}-\x{319f}\x{31c0}-'.
'\x{31cf}\x{3200}-\x{33ff}\x{4dc0}-\x{4dff}\x{a015}\x{a490}-\x{a716}\x{a802}'.
'\x{e000}-\x{f8ff}\x{fb29}\x{fd3e}-\x{fd3f}\x{fdfc}-\x{fdfd}'.
'\x{fd3f}\x{fdfc}-\x{fe6b}\x{feff}-\x{ff0f}\x{ff1a}-\x{ff20}\x{ff3b}-\x{ff40}'.
'\x{ff5b}-\x{ff65}\x{ff70}\x{ff9e}\x{ff9f}\x{ffe0}-\x{fffd}');

So here, I just removed the \x{2e} char (corresponding to the .) to make it work.
I assume it was here for a good reason (or not), so I am basically making the prestashop less safe ?

Prestashop's search class source code

2
  • Prestashop's approach is what Marcus Ranum has called "enumerating badness." It is in general not secure because of the difficulty of being sure you've identified all the badness. As Abe Miessler says in his answer, you get protection against SQL injection by using parameterized queries. (Probably you don't want to tear into the guts of Prestashop to fix this, but you should at least worry about it a bit.)
    – Bob Brown
    Dec 2, 2014 at 10:16
  • 1
    Well, just by getting a bit into prestashop's code gives me pimples... I am fully aware of it. Yet, at least in France, it is the most used ecommerce solution (by 48%). So it is very commonly required for our customers... As a developer, be sure I'm looking for other cleaner solutions to propose
    – Brewal
    Dec 2, 2014 at 10:28

1 Answer 1

6

I don't think it will open you up to SQL Injection.

That said - while I am not an expert on Prestashop or it's code base, I got the impression that the sanitize function you linked to is not just used to prevent SQL Injection, but multiple kinds of vulnerabilities (notice the call to strip_tags on line 102). If this is true you should be careful that you aren't opening yourself up for other types of vulnerabilities. The first one that comes to mind by allowing . is directory traversal.

Ideally, allowing the . character as input wouldn't open you up to this type of attack, but without doing a security audit of the code I can't say for sure.

Also - if they aren't using parameterization for protection from SQL Injection I would be very concerned about the overall security of their code. The concept of rolling your own SQL Injection protection has been discussed at great length in the security community and the consensus is that it is almost never a good idea.

So in summary - I don't think the . opens you up to SQL Injection, watch out for other vulnerabilities and be wary of Pretashop's approach to security.

1
  • Thanks. The string is only used for sql query. That is why I first focused only on sql injection. I am sadly aware of prestashop's security issues or bad coding...
    – Brewal
    Dec 1, 2014 at 18:37

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .