Hacker News Reader

Comment by slovenlyrobot | Hacker News Reader

That's a surprisingly small list, missing e.g. sscanf / gets / strtok / all the other "usual suspects" at least

I don't see a huge problem with strtok as long as it isn't a multi threaded program. It is not in gets territory which is literally impossible to use correctly.

Synchronous reentry is a problem. Say some loop using strtok calls a function... how do you know that that function doesn't use strtok (or call another function that does...).

Any use of non-const static variables in general has this problem, and strtok is just one example.

> Say some loop using strtok calls a function... how do you know that that function doesn't use strtok (or call another function that does...).

I’m constantly surprised that C isn’t specified in such a way that lack-of-reentrancy can be determined at compile time. You’d “just” need a symbol table, sort of like the debug symbol table, in each compiled object, holding for each visible symbol the set of function calls marked `__no_reenter` that are predominated by that symbol in control flow. (Yes, some functions do computed jumps, like with longjmp. Just implicitly label those functions __no_reenter unless the programmer explicitly labels them __reenter!)

The problem isn't exactly reentry. It's that the function, having already exited, has modified global state, and it needs the same state to be there on the next call. If another call is interleaved (another callee with its own strtok loop) then the state is overwritten with the inner loop, and your own next call won't have its expected contents.

Whereas with a term like reentry I think of two strtok frames on the stack at the same time, which is not possible.

I think it's a bit easier to conceptualize with a function that produces a simple return value in a static buffer. Take getpwnam(). It fills a global structure with info about a given user. So you may keep a pointer to that on your stack. Then you call some other function foo(). foo looks up another user. Suddenly you can't rely on that other call to getpwnam() not having overwritten the data in the result you got back the first time.

Ah, yes. What I described wasn't reentry. A reentrant function using strtok could exhibit this problem, but it applies more widely than that.

how do you know that that function doesn't use strtok (or call another function that does...).

The same way you know a lot of other things about the codebase: By reading and understanding it. Given what strtok() is used for, it's almost always going to be working on a single context at any one time.

First, many C projects are libraries that are written for reuse in other programs. So "the codebase" is an arbitrary, unknowable thing. We don't have the ability to look at all the other code that will eventually coexist in the same process.

Second, code changes over time. Assuming you inspect 100 lines of code to ensure that they don't call strtok() ... do you sprinkle comments all over as a warning to future programmers not to add any calls to strtok()?

Finally, assuming we could evaluate all of the code in a program at once, it is impractical to build non-trivial programs that way. Instead, we rely on contracts. The idea is that if a caller conforms to its end of the contract, the callee will conform to its end. This allows us to reason about correctness of some code without reading all the other code in the universe and thinking through all the potential code paths. Any function using strtok() would be presenting a rather ugly contract clause "this function destroys strtok's static state". Even worse, such a clause would be contagious, infecting any client of that function, and so on.

Might as well just use strtok_r everywhere and not worry about if you're using strok safely or not.

"I don't see a huge problem with insert thing here as long as it isn't a multi threaded program."

Usually a bad omen. :)

I can't remember what's wrong with it, but it's responsible for about eleventy billion CVEs over the years

You can safely use gets on trusted input. For example, your program might create a pipe or socketpair to communicate between the processes that result from a call to fork. The stdout of one process feeds into the stdout of the other process.

Unless you have an insane gets implementation that writes the bytes backwards, you can also safely use it on untrusted input. Simply place a guard page at the end of the buffer. You can do the call from a forked child that you let die, taking notice of the problem in the parent. On systems without threading (because of locks in libc) you can do a longjmp to recover.

git does use multiple threads for some longer operations, like gc and pack, so using strtok would be a disaster.

Maybe gets doesn't need to be banned because it got removed in C11 and at least gcc issues a warning on use.

I hope it grows through community effort. This is a good sanity check sort of thing.

I'm curious how sscanf is considered bad?

scanf(“%s”, …) can cause a buffer overflow.

It sucks that there's no scanf("%.*s"), where you give the buffer length as a separate argument. The fact that it's in a string literal also makes using a compile-time constant really ugly.

You can overflow buffers all sorts of ways in C.

Reply on news.ycombinator.com