replace sprintf with snprintf

[dcooley]

Question

The h3ToString function uses sprintf(). Is it possible / worth replacing with snprintf() ?

H3Error H3_EXPORT(h3ToString)(H3Index h, char *str, size_t sz) {
    // An unsigned 64 bit integer will be expressed in at most
    // 16 digits plus 1 for the null terminator.
    if (sz < 17) {
        // Buffer is potentially not large enough.
        return E_MEMORY_BOUNDS;
    }
    sprintf(str, "%" PRIx64, h);   // <-- replace with `snprintf(str, sizeof(str), "%" PRIx64, h);` ?
    return E_SUCCESS;
}

For example, as noted in https://rules.sonarsource.com/c/RSPEC-6069/,

When using sprintf , it’s up to the developer to make sure the size of the buffer to be written to is large enough to avoid buffer overflows. Buffer overflows can cause the program to crash at a minimum. At worst, a carefully crafted overflow can cause malicious code to be executed.

(I'm not a full-on C-programmer, so it may be the case that the sz < 17, and the H3Index h (uint64_t) themselves guarantee this overflow won't occur?)

---

Usecase / Background

I'm building an R library using h3, but upcoming versions of R* won't accept sprintf in compiled code due to the security risk.

* Not strictly R itself, but rather the official repositry of R packagse, CRAN

Use of sprintf and vsprintf is regarded as a potential security risk and warned about on some platforms.82 (including macOS as from version 13).

[isaacbrodsky]

I am fairly confident that this function will not overflow due to our unit and fuzzer tests that try to alert us if this function were to try to write beyond its buffer. It is of course dependent on certain details like single char character encoding. That being said I don't think I could guarantee that there isn't some exotic scenario that I haven't considered.

My recollection from when I looked at this before was that snprintf and related functions were not totally portable. I think some of the incompatibility dealt with edge cases and how failures are signaled, perhaps something that would still allow us to use them?

Another thing that occurred to me is that we might want to offer a compiler flag to simply omit these string functions. I think it's fairly common for bindings to simply replace them with versions directly in the bound language, due to the complexity of marshaling a string between C and managed runtimes. (java, python, and not applicable in JS)

Regarding your code suggestion, sadly I do not think that sizeof(str) is correct. C does not know the actual size of the buffer pointed to by str, so this will actually be the size of the pointer (thus why C has lots of problems with this kind of thing).