Hacker News Reader

Comment by AHTERIX5000 | Hacker News Reader

These can be quite useful when prototyping indeed but please keep in mind that they are not the most robust code around in terms of security and UB if you are actually shipping a product using these.

View on HN

I remember fuzzing stb_truetype and it finding hundreds of trivial segfaults instantly. Perhaps they fixed some but I can't advise using these with any untrusted data, it's obvious security is usually not a priority.

There is now a big fat disclaimer at the top of the file saying basically the same thing:

https://github.com/nothings/stb/blob/master/stb_truetype.h#L...

Ah, that's good, despite being six years late. However, I do not think there is any good reason for code written like this to exist in the first place. Bounds checking is easy and fast (even if less so in C) and things you consider "trusted" can become untrusted easily and quickly turn otherwise minor bugs like file type confusion into critical vulnerabilities.

I mostly agree with you. An important qualifier is that these libraries are meant for games, especially games that package their own font files, and especially games that are on locked-down platforms like consoles and mobile phones. On console games it makes sense to exclude bounds-checking your own data because you want to minimize load times. But yes, bounds checking should have been included and have been on by default with an option to disable it.

I think rust has indirectly shown that the performance impact of bounds checking mostly neglegible in practice. Even in very high performance code, I've never seen anyone turn them off for performance. This makes a lot of sense considering that on modern CPUs everything except cache misses is basically free, so a single unlikely branch compare usually just does not matter.

The only exception I can think of is accessing lots of indexes in a loop, where getting good performance requires rust programmers to insert awkward asserts or casts to fixed length arrays to get the checks to optimize out[1]. But afaik that's mostly because the bound checks impede other loop optimisations like vectorization, not because the checks are slow themselves.

[1] (e.g. when you access indexes 1 to n randomly, assert n < length before)

We have seen some of this; Dropbox has a macro that lets them turn off checks with a feature flag, for example, because it had a noticeable impact.

Ah, that's interesting, I assume this is the link:

https://dropbox.tech/infrastructure/lossless-compression-wit...

Shows a significant improvement in compression code and some examples of checks that can't be elided. I guess this makes sense because it's pretty much the worst case scenario for bounds check impact.

I can still personally say that every time I've blamed rust's bounds checks, I was wrong.

EDIT: neither the unsafe flag nor the macro appear to be present in https://github.com/dropbox/rust-brotli/ anymore today, removed some time in 2017. So it seems they found some other way to deal with it?

Ah interesting! Glad they removed it. I can't quite find out when, and I don't have the time to really dig in right now. Very cool, thanks.

This is unrelated to being single-file libraries though. Any library suffers from the same problem until it's properly fuzzed and fixed.

I didn't get the impression they were saying this is a problem specific to single file libraries but rather it was a problem specifically with the libraries suggested in the linked repository.

Reply on news.ycombinator.com