byterock

It's explain post-ette day there in the Moose-Pen

As I reviewing some of my code for 'Case' statements I had chanch to stop and think about why I was using a Param class for the message on a case. Right now I have something like this;

           {

                left       => { name  => 'Price', },

                right      => { value => '10' },

                operator   => '<',

                message => { value => 'under 10$' }

            },

Before I had that just as a simple string like this

                message => 'under 10$'

Which should be good enough but there is a reason for that change and it goes back to the old bug-bear of aurto generated SQL,

At first I did not think this would be an issue as the SQL would most likely fail syntactically with SQL injection code stuffed into. In any but the final 'When' statement, I would think it would be true, but it is the the final 'When' statement where you could get a vulnerability.

If it was a just a string some very nasty sort could do this;

 {message => “ 1  END; DROP TABLE OrderDetails;” },

and the resulting SQL if this case was used in a 'where' clause would be

SELECT OrderID, Quantity
    FROM OrderDetails;
WHERE price =
 CASE WHEN Price > 10 THEN 0
 ELSE 1  END; DROP TABLE OrderDetails;
END

Even with the above SQL I do not think it would cause much trouble as the statement is syntactically incorrect with the extra 'end' in there but you never know how a DB will react it could actually work on the first, and second parts then fail only on the loose 'END' clause.

Now it is a little bit of overkill on my side as normally anyone using Database::Accessor at the this level could cause havoc without resorting to SQL Injection.

In fact I can think of only one use case where this might happen and that is when there is a requirement for the end user to enter a message, though this is highly unlikely IMHO.

But one is always better off safe rather that sorry.