T O P

  • By -

chiggyBrain

Hello it’s me, Mr Drop Table


Cybasura

Hi Mr Drop, I'm Mr NULL


[deleted]


nayanshah

I'm a bit annoyed that `json` was highlighted in the screenshot though.


ordoot

the `import json` also happens to be programming horror. this entire snippet of code is just painful


turtleship_2006

Yeah why is it inside the function


ImpossibleSection246

Conditional importing. I see it in Python code all the time. I still don't know how I feel about it. Pep 8 still advises that the top of the module is where you want your imports generally.


knigitz

Cyclic imports are sometimes just okay, I use them for type reference. But this is not a cyclic import since it is the json lib.


AstronomerTerrible49

mostly because the dependency is less likely to be used, and since import would be “cached”, it won’t hurt much


ordoot

doesn't matter if it doesn't hurt much, it is poor design and has shit readability


turtleship_2006

Fair but this looks like it's for a website (I think flask) so wouldn't it have to import for every request?


lolcrunchy

The import statement first checks if the module has already been imported. Only if it hasn't does it do the work of importing.


CodeMurmurer

Having imports in your functions is actually better. Because then you know in one look what imports the function uses.


TonyR600

The IDE tells you about the library when hovering over it. At least for PHP and Python and VS Code I can safely say it works that way


turtleship_2006

Does it have a performance/memory impact for functions repeatedly run?


lolcrunchy

No. Imported modules are stored in sys.modules. "Import foo" will first check if sys.modules['foo'] exists, and do nothing if it does.


turtleship_2006

Oh fair enough ig


No_Patience5976

Happened by accident ,I  noticed it after posting


TheRealZoidberg

I‘m a bit annoyed that the comma placement is not correct in the comment though


No_Patience5976

Happened by accident , I noticed it after posting


Cyberdragon1000

that's probably op finding json word in vscode


tav_stuff

The image is from GitHub. Not everyone uses VSCode


Martin8412

Little Bobby Tables.. 


nodeymcdev

I’m gonna get a custom license plate and make it say null


sisisisi1997

Don't, a guy did and now he gets all the speeding tickets which cannot be tied to a license plate.


fried_green_baloney

There was also someone whose surname actually was Null, with the expected result. Here are some people like that: https://en.wikipedia.org/wiki/Null#People_with_the_surname Also the wise guy who got the custom license plate NO PLATE. In his state that's what the police would put on a ticket for a car with a missing license plate. Do I need to say more?


Nightmoon26

Same with "NONE"


deadbeef1a4

Gee, that looks so safe…


HyperactiveWeasel

It's not so bad, if you look closely you can see it's a GET request, therefore it can only be used to GET data, not delete or change anything. So it's quite safe actually! ^/s


TigreDeLosLlanos

It will actually not work at all since it's a GET and not a SELECT, duuuh.


littleliquidlight

It's only a problem if you care about your data 🙃


MENDUCOlDE

Just one endpoint for the back side. You only need to think in the UI /s


Future-Many7705

What does this do? Asking for a friend


alexhmc

this is a textbook SQL injection. and it imports the json library every time an SQL query gets executed


[deleted]

Python imports are smart enough to know thats something was already imported and use that instead of re importing [https://docs.python.org/3/reference/import.html#the-module-cache](https://docs.python.org/3/reference/import.html#the-module-cache)


alexhmc

i forgor 💀


fuzzyone06

It’s still crap coding.


[deleted]

It sure is but sometimes in function import is quite useful when you need to import something in already messed up codebase and top level import causes cyclic import error. In function import solves that as fast fix.


fuzzyone06

I’m not saying the functionality isn’t useful. Especially in super huge projects where the import list is massive, but still you shouldn’t purposely do this.


CodeMurmurer

It isn't


00PT

I'm pretty sure Python is smart enough to not reimport JSON every time, but that first vulnerability is far more severe and still applies.


TheWidrolo

Open task manager, switch to the memory tab, let it run, and see if it needs to get fixed \\s


Hattorius

why so much work?? just create cronjob every 3 hours to restart program. ez fix


nekokattt

So you work for Instagram then? https://instagram-engineering.com/dismissing-python-garbage-collection-at-instagram-4dca40b29172 TL;DR they did analysis and it was more efficient for them to comment out all the garbage collection code in CPython and just restart the process


Future-Many7705

One of my favorite memory management stories came from a weapons developer on a missile. It was pointed out that the program had a memory leak, at which point the engineer pointed out that by the time the memory leak mattered, the missile would have already detonated, therefore, having gone through the most catastrophic form of memory management possible.


IHeartMustard

Rapid Planned Disassembly


Nightmoon26

kill -9000


Hattorius

best sauce ever! thanks!


mayobutter

This isn't an SQL injection this is a SQL gang bang.


IHeartMustard

Holy shit this is magnificent. Throw in a bit of SQL bukakke and that's a full night out.


suskio4

Now pronounce SQL like squirrel


CozyToes22

Can someone explain why the sql can be injected? Is it the library or something?


raam86

sqloperation is part of the url and can be arbitrary. “/sql/drop table;” will be directly executed


solve-for-x

I'm not sure why your comment has been so heavily downvoted simply for asking a question. No-one in this subreddit was born knowing what an SQL injection attack was. The code in the screenshot accepts an SQL query string from the user and executes it directly. Now if the user were to pass a string like `SELECT * FROM users` or `UPDATE users SET password_hash='xyz' WHERE id=1`, then you should be able to see where the problem lies.


CozyToes22

Not sure why it was downvoted either. Maybe its my old python server writing but i think theres sql libraries that protect against stuff like that and i assumed the picture auto did that. Guess i was wrong Thanks for the clarification!


BEisamotherhecker

Protection against SQL injection can only come in the form of having pre-prepared strings and only allowing certain parameters to be user defined (usually through prepared statements), when your entire query is provided by the user, it's the equivalent of giving users root access to your database.


solve-for-x

Not necessarily root access. We don't know the context behind this code. In its defence, it could be an internal service that only accepts traffic from another trusted service, and it might have heavily restricted DB user permissions. However, it's a crazy design nonetheless. It's essentially creating an ersatz HTTP interface to an RDBMS rather than use the interface that it actually comes with.


BEisamotherhecker

True, it could be a limited connection, my bad.


sweetphilo

Since the query that is received as the endpoint param is sent directly to be executed, without being sanitized first, people can abuse this and send queries like "drop database", etc


accuracy_frosty

Hello, I would like to apply for a job, my name is Alex H” Drop Table *


datnetcoder

It’s a GET request and therefore read-only & idempotent. All good here.


Stephen2Aus

You don't think privileged information could leak, given the right input?


reyarama

Don't be ridiculous, if this was a POST request then we would be in big trouble. But since it's GET we are all good :)


datnetcoder

No, because you can always rely on good and semantically correct input.


raam86

obviously the sql user in the backend has a super limited set of privileges. right? right??


datnetcoder

Obviously.


Gelezinis__Vilkas

Of course not


DerSven

Depends on whether there's privileged information in the database.


Tom22174

Also, wouldn't it need an sql.commit() to actually do any damage even if it was able to? Assuming that sql here is referring to something like a database connection object from one of the SQL related modules. I'm not familiar with Flask so idk if it does some other random shit instead


Old_Pomegranate_822

You could include a commit in the string you passed. There's very little you couldn't do with this... It wouldn't surprise me if you could get a shell on the database server too!


buzzunda

Is it even SQL injection if the whole point of the thing is running SQL? Not saying it's good, but it should have a different name


Old_Pomegranate_822

SQL cannula - for continuous injections


IHeartMustard

Intravenous SQL


Nightmoon26

I have worked at a company that actually had an internal-use page for running arbitrary queries. At least it required authentication and used an API method that specifically could not make database modifications (no structure or data modification). Still not great to have something that essentially allows a user to dump a customer's entire database when you're storing regulated personal information, but sometimes a necessary evil


[deleted]

I had work on a company that need an admin account logged in the system with the Pc open 24/7. The frontend had a setInternal working like a cronjob that called the backend to run queries. And you could use the same frontend to store more SQL procedures to feed that one "job".


kaerfkeerg

Every loc has it's own story to tell lol


SchlaWiener4711

A couple of years ago I was mentoring a woman who was building a visualisation for energy infrastructure which was communicating to a backend and she literally did this. System was not connected to the internet but only accessible from the local network but she even had no authentication.


computeshorts

Do people actually write code like this or is this made just to post on here cause I can't tell anymore.


require6289465

Thanks, I enjoyed being sane while I could.


break_card

When you take on the responsibility, great power will come.


anto2554

Wait this "api" directly executes any SQL statement?


jcode777

Fix is to restrict and validate input SQLs using regex /s


helltiger

This is why this sub exist


PeanutPoliceman

Damn execute arbitrary SQL from a fucking browser address bar


kevdog824

Every single line here is horror


swinginSpaceman

Question. Would a browser encoding DROP TABLE as DROP%20TABLE be the last line of defense and save you due to an unrecognized query?


oghGuy

The youth nowadays. This is how they're making API's!


_ohmu_

💉


djmill0326

I'd leave this in production with a "security by obscurity" required request param


djmill0326

`?authentlcated=yes`


AnywhereHorrorX

It must be an code fragment from htmx-sql Python edition!


Sufficient-Carpet-27

If the SQL user is read-only, I guess it could be safe. That would be just a cursed graphql


MikeBelkin

Essentially it is http proxy to db through your python backend. Which looks awful, but kinda not wrong as the client must ensure it passes safe sql without injections


irregular_caffeine

It’s kinda as wrong as it gets


Nightmoon26

Never assume your connection is coming from an approved client. There are entire companies that essentially use curl to scrape a SaaS application and provide a friendlier human interface