Hacker News Reader

Hacker News

Comment by efnor | Hacker News Reader

Parent Full thread

efnor

Judging by the code, everything that goes into the database is a Base64 encoded. Where do you see a buffer overflow?

View on HN

Long URLs make it crash. Really. Just try and put in a very long URL...

https://github.com/riolet/longs/blob/master/longs.c#L238 looks like an example of a buffer overflow.

Etc.

SIZE is defined as 2048 bytes (line 59). Firefox can do GET requests with 8KiB URLs.

The 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.

Look down one top-level comment on this thread. There's an extremely straightforward heap overflow exposed in simple request processing.

Dependent on what comes out of the DB, not what goes in.

That 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.

That 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?(⸮)

I'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.)

It'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.

I 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?

You'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.

You 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.

Yes, I wasn't contradicting that. I was explaining what the issue was.

Reply on news.ycombinator.com