Jan Steinman wrote:

...

From: Brion Vibber brion@pobox.com This snippet appears to be vulnerable to SQL injection attacks. A cleverly written signature or other option on the model row could probably be used to overwrite everyone else's passwords or such.

Since this is used for copying *one* user's preferences to all the other users, and assuming you'd be pretty careful about selecting that one to copy, and also assuming that this script would only be run by trusted individuals in a trusted environment, what's the problem?

That's a lot of assumptions -- *that* is the first problem.

At a minimum, it can break with an SQL error if they make a mistake and stick an apostrophe in one of the preference fields. At a maximum, they can have their database compromised if they "choose poorly".

The second problem is that it's an example of a bad coding habit; if somebody's got one example of this kind of code, they need to look around in their other code to make sure they don't have a dozen others, which might be even more exploitable.

The third problem is that they're showing this code to other people as a potential solution to a problem, which is going to show up when people with a similar problem search the archives. Now your assumptions have to hold not just for the first group, but for anyone who copies their code or uses it as an example for writing their own code to solve another problem.

-- brion vibber (brion @ pobox.com)