On Wed, 16 Nov 2022 07:03:12 GMT, Xue-Lei Andrew Fan <[email protected]> wrote:
>> Hi, >> >> May I have this update reviewed? >> >> The sprintf is deprecated in Xcode 14 because of security concerns, and the >> use of it causing building failure. The build could pass if warnings are >> disabled for codes that use sprintf method. For the long run, the sprintf >> could be replaced with snprintf. This patch is trying to check if snprintf >> could be used. >> >> Thanks, >> Xuelei > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > address review comments I don't think it is safe to use the return value of `os::snprintf` to update buffer positions. `os::snprintf()` calls `os::vsnprintf()` which, at least on Posix, returns the return value of `vnsprintf(3)` verbatim. If native `vsnprintf(3)` conforms to SUSv2 [1], it will return <0 if the buffer size is zero. So for cases where we calculate the buffer size based on what was written, and we are just at the edge of the buffer, we would move the next write position backward. But much worse: if the native `vsnprintf(3)` conforms to C99 (e.g. glibc, BSD libc) they return "number of characters (not including the terminating null character) which would have been written to buffer if bufsz was ignored". [2] So, if the buffer was too small and we truncated, and we use the return value to calculate the next write position, we will write outside the buffer boundaries. Regardless of what behavior we have - C99 or SUSv2 - we cannot just use the return value to update the next write position without first checking the return value. We could - and probably should - decide on C99 or SUSv2 behavior for all platforms, and modify `os::snprintf()` to provide that in an OS-independent way. But unless we decide on some mongrel of both C99 and SUSv2 behaviors, the problem remains. Cheers, Thomas [1] https://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html [2] https://en.cppreference.com/w/c/io/fprintf ------------- PR: https://git.openjdk.org/jdk/pull/11115
