conversion from string to JSON is not multithread-safe about jansson HOT 5 CLOSED

The cause is that the conversion from string to double, in strconv.c: jsonp_strtod, uses the localeconv() function, which is not multithread-safe (see POSIX https://pubs.opengroup.org/onlinepubs/9699919799/functions/localeconv.html ). In particular, in glibc, localeconv() returns a pointer to a static variable.

So, in thread1 with its English locale, localeconv()->decimal_point is ".", whereas in thread2 with its French locale, localeconv()->decimal_point is ",".

In thread1, when jsonp_strtod is called on "1.5", it first calls to_locale(strbuffer). Here, it can happen that the return value of localeconv() is disturbed by thread2, such that localeconv()->decimal_point returns ",". In this case, to_locale produces the string "1,5". When strtod is called on this string, it returns the number 1, and the end pointer points to the comma (rather than to the end of the string).

from jansson.

The fix is to use sprintf (buffer, "%#.0f", 1.0) instead of localeconv(). sprintf is multithread-safe.

from jansson.

Thanks for the detailed bug report @bhaible! Do you think #677 would be the correct fix?

from jansson.

Do you think #677 would be the correct fix?

src/strconv.c: The code change is correct, yes. But I would keep the comment in lines 17..25, since it is still valid and relevant even after this fix.

test/bin/json_process.c, test/suites/api/util.h: These changes needlessly reduce the test coverage. Before, with the setlocale (LC_ALL, "") invocation, the test would use the "C" locale or an English locale on some machines, and a French or German locale (with a comma as decimal-point character) on other machines. If you remove the setlocale (LC_ALL, "") invocation, the test uses the "C" locale always; this provides less coverage of relevant test scenarios.

from jansson.

Thanks! Fixed in #677.

from jansson.