New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use of snprintf and vsnprintf in Curl #3296
Comments
Since the function returns another return value I think I would prefer to change the function name in all places so that we can check it and fail if the wrong function name is used when code gets changed in the future and possibly start to use the return code. I'll file a PR. |
The function does not return the same value as snprintf() normally does, so readers may be mislead into thinking the code works differently than it actually does. A different function name makes this easier to detect. This also adds snprintf() and vsnprintf() is "banned" functions in checksrc to make us not mistakenly use them going forward. Reported-by: Tomas Hoger Assisted-by: Daniel Gustafsson Fixes #3296
The function does not return the same value as snprintf() normally does, so readers may be mislead into thinking the code works differently than it actually does. A different function name makes this easier to detect. Reported-by: Tomas Hoger Assisted-by: Daniel Gustafsson Fixes #3296
One more observation regarding Repeating the test from: http://demin.ws/blog/english/2013/01/28/use-snprintf-on-different-platforms/ for curl, the output is:
|
The curl code base use
snprintf
orvsnprintf
functions on multiple places. However - just like for all printf family functions - it uses its own implementation of those functions -curl_msnprintf
andcurl_mvsnprintf
- which is achieved via macro definitions incurl_printf.h
:https://github.com/curl/curl/blob/curl-7_62_0/lib/curl_printf.h
However, there is a notable difference in what
curl_m*snprintf
functions return compared to what's defined for*snprintf
in POSIX or C99 for the case when destination buffer is too small and the output is truncated:Looking at few cases where
snprintf
return value is not ignored, the curl code expects and relies on the curl implementation return values. There are often no checks to ensure that the return value does not exceed destination buffer size, and the value is directly used to increment the pointer that is used for subsequent writes, or decrement free space counter. That could lead to out-of-bounds buffer access and integer underflow if POSIX snprintf was used instead. An example of such code pattern can be found in e.g.curl_version()
:https://github.com/curl/curl/blob/curl-7_62_0/lib/version.c#L129-L131
This can be a problem if
*snprintf
implementation used by curl is to be changed in the future, or if anyone tries to make curl use printf functions from their system C library instead of curl implementations.This can also cause confusion to folks from outside of the Curl project who review or contribute to the code of the project.
The explicit use of
curl_m*snprintf
functions should be considered for places where non-standard return values are expected.The text was updated successfully, but these errors were encountered: