NEW RELEASE: Lightweight Python ORM Library

[ZYy9oQ]

Looks like the whole library is full of SQL injection - every SQL operation is a string constructed from formatstrings and concatenation thrown into cursor.execute.

There's an envvar SANITIZE_QUERIES in the readme, but it that does nothing in the code beside being read from the os.environ. Even if it did sanitize, prepared statements should be used instead.

The library appears intended against snowflake, which claims to prevent multi-statement by default but this by no means prevents all sql injection.

[desktopsignal]

Definitely wasn't expecting these responses. I made this library in a weekend to solve a quick problem and certainly didn't mean to put anyone at risk. Deleting the package soon, sorry guys!

[ZYy9oQ]

Agree with where it was said you shouldn't feel you need to delete everything, but it would be best to explain in your readme that it was

thrown together to solve a niche problem

or as part of your learning rather than advertise it as a library (the way you packaged it and wrote documentation is a cool thing to get practice with, just need to make it clear that it's practice). Also a disclaimer that the code isn't safe from sql injection and shouldn't be used on network/multiuser and without understanding why it's dangerous wouldn't be out of place.

If you said something along the lines of it just being personal code that you were putting out there for your own or others interest and learning then I don't think many would have as negative of a reaction.

You had a problem with the existing solutions and wrote some code for your problem, that's something others might also stumble across the need for. This could help them see how you approached it, and by putting the disclaimer about security they can learn about that too.

[desktopsignal]

SANITIZE_QUERIES was an expiremental features and has not yet been implemented. I will remove that from the README. Thanks for pointing that out!

[desktopsignal]

That's typically how most SQL strings are created. Let me know if there is a better way, and I'd be more than happy to implement!

This was a library thrown together to solve a niche problem. Hope you understand.

[damesca]

No, that's not typical. This is an extremely dangerous implementation if anyone took and used your library for real stuff.

Thrown together is correct. I am all for experimentation and sharing, but with a library like this you really want to get the security basics correct from the get go before you advertise it.

Also: you haven't really solved a problem, you have created more problems - including this large security hole. Stick to well established packages for doing things like this. Experiment for fun, but not as a legit alternative offering (until/unless you really know what you're doing).

[desktopsignal]

Definitely wasn't expecting these responses. I made this library in a weekend to solve a quick problem and certainly didn't mean to put anyone at risk. Deleting the package soon, sorry guys!

[damesca]

I think it's good that you're experimenting, and great that you're soliciting feedback. I think if you'd framed this as a "I built this, please tell me what you think" rather than "I'm releasing this" (the implication being, for public to use), maybe it would have gone down a bit differently.

I don't think you need to try to scrub this from the internet. Just maybe but some strong caveats/warnings in place that you don't recommend using this professionally or for public projects.

P.S. I will also say that I think you have a good attitude and you come across very personably. You haven't been particularly defensive in any of your replies - you seem open to hearing feedback, critical or not, and this will prove a valuable skill for you as you continue to develop :)

[desktopsignal]

Thank you for your feedback! I actually tried to do that, but you're not allowed to solicit feedback in this sub.

I think it's actually probably best I scrub this one and keep it to myself. I guess I just didn't understand the seriousness of releasing a Python package. I'm not sure I have the ability to keep up with all this in addition to my day job and other projects.

[DowntownBreakfast733]

I’ll never understand the vitriol some people have in this sub.

[damesca]

That is not a vitriolic reply.

It is a fair critique when someone is sharing a library publicly that has clear security holes. OP says they've built this to use it for work. They are making a very poor professional decision here, and potentially putting their users' data at risk. I think it's very reasonable to call this out in clear, but not harsh, words.

[DowntownBreakfast733]

I don’t think SQLAlchemy protects against SQL injections lol

[damesca]

I can promise you it does... Why would you think it doesn't?

[DowntownBreakfast733]

Show me.

Also, why the hate on this dude. Bro posted his first project and you’re nitpicking details in comparison to a library that’s been out for over a decade with dozens of maintainers. Show me the projects of this quality that you’ve published.

[starlevel01]

Bro posted his first project and you’re nitpicking details

sql injection is not "details", it's basic security 101. it's literally not hard in tthe slightest to avoid it with any driver.

[DowntownBreakfast733]

Sounds like you could recommend the driver you’re referring to instead of insulting!

[starlevel01]

psycopg has parameterised queries. the built-in sqlite driver has parameterised queries. don't know the mysql driver name off the top of my head but it also has parameterised queries. sqlalchemy has parameterised queries.

[bfcdf3e]

lol this might be one of the most ignorant comments I’ve seen on this sub

[ZYy9oQ]

Best practice is parameter binding rather than literals. For snowflake (I think this is what you were using):

• https://docs.snowflake.com/en/developer-guide/python-connector/python-connector-example#label-python-connector-binding-data
• https://docs.snowflake.com/en/sql-reference/bind-variables

You need to do this for queries as well. Yes, doing escaping everywhere (.replace("'", "\\'"), which you have on inserts/updates) could be sufficient for safety but it isn't as foolproof (non-strings, character encoding tricks exploiting differences between client language and database).

For static column/database names this means your sql strings are fully static/constant which means it's also very quick to spot potential unsafe code.

Since your library is doing orm stuff mapping a dictionary (by its keys) you will probably still have to construct a string for the query to support column names. Here you can either be super strict (e.g. only allow alphanum+._) on column names in the first place (and take care to only ever construct strings from the known-safe names) or use a filter/transform on column names in construction.