Hacker News Reader

Comment by h2337 | Hacker News Reader

What's insecure? Can you explain what's the vulnerability here and how and by whom can it be exploited?

Assuming that code is actually present in your app, env vars can hold more than 255 characters. Easy buffer overflow to trigger. Use length-bounded copies and concats...

That's just off the top of my head; I've not written in C in a while.

Why would you want to trigger a buffer overflow in user application if you can already control HOME envvar?

Yeah, that is not a helpful attitude to take when it comes to this sort of thing. If nothing else, a super-long home path can crash your app and leave your user scratching their head. In other words, this is a bug (as is the fact that paths are not necessarily limited to 255 characters in the first place; see the PATH_MAX constant, I think it is?).

As to what could be accomplished with an overflow? I don't know; I'm not in security, and I don't sit around thinking of possible uses for various bugs when it comes to compromising systems.

Perhaps the most important thing to realize, though, is that you're distributing software publicly. Your security situation may not be the same as your user's security situation. Assumptions should not be made.

Something to keep in mind.

Thanks for the discussion. Fix is already committed.

No problem. =)

As long as you’re fixing that bug, you should do it right. If the return value from snprintf if more than 256 but less than a few GB then you should malloc a buffer big enough to hold the string and then call snprintf again with the new buffer. Only if that or malloc fails would you print an error. (It’s really a shame that the C standard library requires so many extra steps to do things correctly; this ought to be way easier.)

Not sure offhand how portable it is, but asprintf() handles automatic buffer allocation, thus not requiring any extra steps afaik.

It does exit on MacOS and Linux, at the very least.

Those are so unportable that I’d completely forgotten about them :)

But my man pages say that they exist on BSD in addition to GNU, so that’s pretty good these days. I say go for it.

Using strcat to a fixed size buffer is like using a gun to kill flies in a crowded flophouse while on crystal meth.

Basically, any path longer than 256 characters for `mapFilename` would cause a buffer overrun.

An unprivileged app could run your app (say, with more privileges), with a very long `HOME` environment path, causing a buffer overflow, and potentially exploit it to use your app's privileges to do more stuff than it was supposed to.

Basically, you should never use strcpy and strcat and but use the secure alternatives like strcpy_s and strcat_s, even when you know the source buffer would never exceed the destination size.

> (say, with more privileges)

Isn't it a moot point if unprivileged app can already run anything with more privileges? In normal operation, connmap requires no special privileges.

Sure, but since there's no enforced standard for how privileges are configured on a system, there's always the possibility that your app to be the only escape ticket.

You can dismiss that possibility of course. But, as a general habit, it's best to use secure alternatives instead of mulling over probabilities every other line.

As a positive side-effect, the change would make your app not crash on systems with long HOME env paths.:)

I see you already addressed it but here let me give a scenario.

Say the program was installed and set so the user didn't have privs to modify the executable (so an attacker couldn't just change it to do what they want).

A buffer overflow could allow an attacker to gain control flow of the program and feed bogus data to the user allowing them to scrub their presence from the map.

Also, awesome project!

Reply on news.ycombinator.com