5 ms·

Parameters require you to be well-behaved with using them consistently. I was thinking of disallowing the use of quotes in queries unless you use an 'unsafe' me

by Joeri 14y ago

Parameters require you to be well-behaved with using them consistently. I was thinking of disallowing the use of quotes in queries unless you use an 'unsafe' method (naming prefix). Still have to see how it would pan out.

wvenable 14y ago
Disallowing quotes isn't a half-bad idea but it won't catch everything; one could still SQL-inject into a query that expects an non-parameterized numeric value. I prefer to abstract query building enough that most of the time I'm not constructing query text directly.
kijin 14y ago
> Parameters require you to be well-behaved with using them consistently. That's the whole point! Since it looks like O.php was intended as a framework for new projects rather than integration into existing code base, you could just tell people that they should use parameters when using O.php. No legacy code to support = all queries will be written from scratch anyway. Please, don't reinvent the wheel of preventing SQL injection. It's a solved problem, and solving it again is boring. Bringing a bit of sanity to PHP, on the other hand, can be a much more exciting task.
- Joeri 14y ago
  Murphy's law applies. If there is a way to do it wrong, people will do it wrong. There's a reason SQL injection remains in the OWASP Top Ten. I disagree that it is a solved problem. The design of O is in part based on my experience using Zend Framework on large projects. I've learned that the only way to get people to write good code is not to educate them on the right way, but to make the right way the default / easy / lazy way. That's the issue with PHP as-is. It's possible to write good code in it, but it is decidedly not the default. But, I see now that I will first ship what's in O already in a way that is reusable on other projects, mostly because I want to start mixing it with my own production code as well. After that I'll circle back to PDO.
  - kijin 14y ago
    > the only way to get people to write good code is not to educate them on the right way, but to make the right way the default / easy / lazy way. Completely agreed. Binding each parameter manually, or even writing a simple prepare/execute pair, is probably too much hassle for somebody who is used to the utter simplicity of a mysql_query() function call. Which is why, in one of my own home-baked libraries, I hide all of the prepare/bind/execute complexity behind a single method call: $rows = DB::query('SELECT * FROM table WHERE col1 = ? AND col2 > ?', $param1, $param2); Behind the scenes, it's PDO with prepared statements. On the surface, it's just as simple as mysql_query(), except you don't even have to worry about string interpolation. You can also pass the parameters as an array if you want to. I'm not saying that this is the best way to do it, but if the way you're planning to do it is anything that looks remotely similar (prepared statements and bound parameters behind the scenes), then you have my apologies for having been too cynical without seeing the actual code. On the other hand, if it's just a sanitizer based on a blacklist of special characters, I would persist in my criticisms.
    - Joeri 14y ago
      In the codebase i work on professionally we use zend_db for a similar approach, except we use named parameters (on oracle). It works well until you have to construct complex queries, and then the temptation to start concatenating quoted values into the query is overpowering. I will stew a bit more on what approach (if any) is best for queries.

by Joeri 14y ago

wvenable 14y ago

Disallowing quotes isn't a half-bad idea but it won't catch everything; one could still SQL-inject into a query that expects an non-parameterized numeric value. I prefer to abstract query building enough that most of the time I'm not constructing query text directly.

kijin 14y ago

> Parameters require you to be well-behaved with using them consistently. That's the whole point! Since it looks like O.php was intended as a framework for new projects rather than integration into existing code base, you could just tell people that they should use parameters when using O.php. No legacy code to support = all queries will be written from scratch anyway. Please, don't reinvent the wheel of preventing SQL injection. It's a solved problem, and solving it again is boring. Bringing a bit of sanity to PHP, on the other hand, can be a much more exciting task.

Joeri 14y ago
Murphy's law applies. If there is a way to do it wrong, people will do it wrong. There's a reason SQL injection remains in the OWASP Top Ten. I disagree that it is a solved problem. The design of O is in part based on my experience using Zend Framework on large projects. I've learned that the only way to get people to write good code is not to educate them on the right way, but to make the right way the default / easy / lazy way. That's the issue with PHP as-is. It's possible to write good code in it, but it is decidedly not the default. But, I see now that I will first ship what's in O already in a way that is reusable on other projects, mostly because I want to start mixing it with my own production code as well. After that I'll circle back to PDO.
- kijin 14y ago
  > the only way to get people to write good code is not to educate them on the right way, but to make the right way the default / easy / lazy way. Completely agreed. Binding each parameter manually, or even writing a simple prepare/execute pair, is probably too much hassle for somebody who is used to the utter simplicity of a mysql_query() function call. Which is why, in one of my own home-baked libraries, I hide all of the prepare/bind/execute complexity behind a single method call: $rows = DB::query('SELECT * FROM table WHERE col1 = ? AND col2 > ?', $param1, $param2); Behind the scenes, it's PDO with prepared statements. On the surface, it's just as simple as mysql_query(), except you don't even have to worry about string interpolation. You can also pass the parameters as an array if you want to. I'm not saying that this is the best way to do it, but if the way you're planning to do it is anything that looks remotely similar (prepared statements and bound parameters behind the scenes), then you have my apologies for having been too cynical without seeing the actual code. On the other hand, if it's just a sanitizer based on a blacklist of special characters, I would persist in my criticisms.
  - Joeri 14y ago
    In the codebase i work on professionally we use zend_db for a similar approach, except we use named parameters (on oracle). It works well until you have to construct complex queries, and then the temptation to start concatenating quoted values into the query is overpowering. I will stew a bit more on what approach (if any) is best for queries.