5 ms·
Looking at the coding patterns used in the C source, I am utterly horrified this is running live on a public facing website. I can see at least one buffer over
by pslam 10y ago
Looking at the coding patterns used in the C source, I am utterly horrified this is running live on a public facing website.
I can see at least one buffer overrun dependent on database contents, and I wouldn't be surprised if there's public-facing vulnerabilities in this thing, but I don't want to spend another 5 minutes looking.
- efnor 10y agoJudging by the code, everything that goes into the database is a Base64 encoded. Where do you see a buffer overflow?
- hueving 10y agoDependent on what comes out of the DB, not what goes in.
- zeta0134 10y agoThat still counts as a vulnerability, even if it's not exploitable without also being able to write the correct state into the database. Databases almost always contain user-generated data. I don't trust the data contained therein any more than I trust the user. Validate it every time.
- themihai 10y agoThat sounds a bit too much. The data from db is supposed to be already validated. Do you validate the input received from the validation function as well?(⸮)
- zeta0134 10y agoI'm not saying you need to fully validate the database, but at least making sure the row you're about to read can actually fit in the buffer you just allocated would be a good practice, even if the input validation shouldn't normally allow it. If the database could theoretically hold it according to its schema, the program should be prepared for the largest row size that is still technically legal. (Where "prepared" may simply mean that it throws a "This should never happen" error and aborts the request.)
- ifoundthetao 10y agoIt's a second stage SQL injection, and it's very serious. Here's a contrived example: At the user creation page the malicious user wants to get admin access. They have their username be: admin' -- They do this because they are betting on a user named 'admin'. Their password is: alwaysSanitizeInputs So, a user is created named "admin' --". Now, they go in to update their password. The DB nicely pulls out their username, "admin' --", and it says: UPDATE users SET password = SHA1('newPassword') WHERE username = 'admin' -- AND password = SHA1('alwaysSanitizeInputs'); So what happens? Well, the user "admin" now has a different password, and the Malicious user knows what it is. That's why you still need to sanitize.
- themihai 10y agoI don't think the solution is to validate it everytime you use that data/value. I fail to see how you mitigate the issue with the second validation. You have duplicate records so hoe you fix it? Shouldn't you check duplicate sccounts on sign up?
- ifoundthetao 10y agoYou're still thinking too small. It isn't about validation for duplicate accounts, and by the way, this gets past that duplicate account validation, because "admin" is different from "admin' --" You get around it by parameterizing your queries, and not blindly trusting data that comes from the database.
- hueving 10y agoYou don't have to fully validate it, just don't fail so spectacularily that it corrupts the heap or crashes the whole program. Ideally a corrupt record only breaks the requests that read that record with an HTTP 500 or something similar.
- hueving 10y agoYes, I wasn't contradicting that. I was explaining what the issue was.
- willvarfar 10y agoLong URLs make it crash. Really. Just try and put in a very long URL... https://github.com/riolet/longs/blob/master/longs.c#L238 https://github.com/riolet/longs/blob/master/longs.c#L238 looks like an example of a buffer overflow. Etc.
- snerbles 10y agoSIZE is defined as 2048 bytes (line 59). Firefox can do GET requests with 8KiB URLs.
- willvarfar 10y agoThe URL is base64-encoded before they try and fit it in a 2K buffer, so the the longest URL it can handle is actually shorter than 2K.
- tptacek 10y agoLook down one top-level comment on this thread. There's an extremely straightforward heap overflow exposed in simple request processing.
- 0xdeadbeefbabe 10y agoDoes anyone else think pslam is ruining the horror genre?