Hacker News Reader

Comment by janmo | Hacker News Reader

I worked for a company and we used the PHP ORM Propel. So in theory no SQL injections you would think, WRONG.

We used a function like findOne() (I don't recall exactly). It looked like this:

$resetTokens->findOne($GET['password-reset-token']);

The issue was that findOne would accept wildcards, so one could use ?password-reset-token=% in the URL and reset the password of any random users.

View on HN

I think the webpage is talking specifically about Laravel. It ambiguously doesn't mention Laravel till later, but the code snippet looks like Laravel code. Laravel's ORM does sanitise strings.

The snippet also validates request inputs, so clearly it doesn't assume that inputs are safe.

I totally agree with everyone before me here on the issue of security.

If an app stands the stress test against say for example this comprehensive list(1), it can consider itself somewhat safe or at least benchmarked. Otherwise, only vague and unsubstantiated claims, which does not help PHP nor any other programming language or framework.

[1] https://github.com/payloadbox/xss-payload-list

In both Symfony and Laravel these days they have their own request objects to help you get information on the request. You shouldn’t be reaching into the get or post variables directly like that.

i.e. $request->query(‘password-reset-token’);

... why would you pass $GET through to a logical layer with access to a data store write without sanitizing it?

This seems like a pretty basic thing to fix, but then I only have your snippet to go by.

That’s a parametrized API that’s supposed to be safe against injection, at least to anyone who’s ever used parametrized APIs and hasn’t read the documentation of this particular library in detail. That it supports wildcard makes as much sense as log4j executing code in textual messages.

If an ORM/builder casually puts =/IS and LIKE in the same method, don’t touch it.

Reply on news.ycombinator.com