DbManager - recommended or unsafe?

[JohnnyD]

I'm not an expert at these things, but aren't the helper methods like executeQuery() dangerous for SQL injection attacks? If so, then should they really be provided - of course they are perhaps aimed at newbies but on the other hand those are the kind of people who are not going to be aware of the risks so my question is whether SFS should make it so easy for people to introduce security holes in their apps.

Not intending this as a criticism, but a real question/discussion.

[rjgtav]

Hi,
I'm sorry but I'm not understanding how will the executeQuery() and executeUpdate() methods help on SQL injections.
These methods are only available on the server-side, which means that only a server-side extension, which is created by the application's developer, is able to connect and interact with the Database. An user can't directly access the database through the DBManager.
You can also use the _server.escapeQuotes() method to remove any quote that may appear in a message sent from an user before saving it directly to the database.

[JohnnyD]

It would be unsafe because anytime you build a SELECT statement as a simple string, you are vulnerable... it's likely you will use values entered by the client to build that string. e.g. logins (the old Bobby Tables thing ) or any time you let a user do a search for something.

It doesn't matter the class is on the server, injection vulnerability has nothing to do with the client having direct access but simply means it's vulnerable to malicious users and naive developers. Best practices in Java is to use parameter binding, rather than manually trying to remove any bad characters from the input string... but even then I'd suggest something like escapeQuotes should be used by default to protect inexpereinced developers (let's face it, game developers are not always best at server security).

[rjgtav]

Removing bad characters from the messages sent from the client covers most of the SQL injection scenarios.

If you prefer to use Prepared Statements, you can easily get the JDBC connection trough the DBManager.getConnection() method, which will give you direct access to the JDBC framework. Don't forget to always close() the connection after using it, otherwise you will exhaust the pool.

You can also use ORM (Object Relational Mapping) like Hibernate.
You can easily access Java objects from an AS extension, check this chapter of the docs in order to learn more about it.

I also suggest you to use the Search tool of the forums, as this topic has already been discussed sometimes in the past.
I've found these topics:
search.php?keywords=sql+injection&fid%5B0%5D=4
viewtopic.php?f=4&t=5928&p=24443&hilit=sql+injection#p24443
viewtopic.php?f=4&t=1756&p=7015&hilit=sql+injection#p7015

[JohnnyD]

Yes - I'm aware of those. My point is that since SFS provides easy 'direct SQL' methods, other people will just use them and never realise the risk. I don't know how you'd change it though... since removing those methods would make it more difficult.

Perhaps simply add a warning to the JavaDoc/manual.

Anyway, I just wanted to raise the point - not to insist you change anything! Thanks.

[Lapo]

If you're unaware of SQL injections and you trust the client input without checking at least the critical parameter then, yes there could be security issues.
The same could be said for the fact that there are "easy ways" to send custom server requests, so a naive programmer could let all sorts of un-sanitized parameters harm the server logic.

All it takes is a bit of common sense
If you don't like the "easy way" you're free to grab the connection manually and bypass those calls.

Cheers

[JohnnyD]

Yeah but the people who know how to do that are the ones who know the dangers of not doing it
SQL injection-proofing isn't common sense in my view, otherwise it wouldn't be such an endemic problem!

A note in the javadoc would still be worthwhile IMO but I'll not argue it any further

[Lapo]

Sure, makes perfect sense
Thanks

[Lapo]

I'm not an expert at these things, but aren't the helper methods like executeQuery() dangerous for SQL injection attacks? If so, then should they really be provided - of course they are perhaps aimed at newbies but on the other hand those are the kind of people who are not going to be aware of the risks so my question is whether SFS should make it so easy for people to introduce security holes in their apps.
Not intending this as a criticism, but a real question/discussion.

I am reviving this thread to point out that actually the SFS2XDBManager class already provides the "secure" versions of executeQuery() and executeUpdate() methods. In fact there's and overloaded variant for each one that accepts a SQL statement with placeholders and an array of parameters.

Behind the scenes a PreparedStatement is used, which takes care of sanitizing the parameters.

Sorry for not having pointed out this before.
Thanks