[tor-bugs] #31567 [Applications/Tor Browser]: NS_tsnprintf() does not handle %s correctly on Windows

[Tor Bug Tracker & Wiki]

#31567: NS_tsnprintf() does not handle %s correctly on Windows
-------------------------------------------------+-------------------------
 Reporter:  mcs                                  |          Owner:  gk
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Critical                             |     Resolution:
 Keywords:  ff68-esr, tbb-9.0-must-alpha,        |  Actual Points:
  TorBrowserTeam201908R                          |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by pospeselr):

 * keywords:  ff68-esr, tbb-9.0-must-alpha, TorBrowserTeam201908 =>
     ff68-esr, tbb-9.0-must-alpha, TorBrowserTeam201908R
 * status:  needs_information => needs_review


Comment:

 **tldr** there's a patch at the end

 Did some digging on this tonight just for fun. According to this post on
 stack overflow ( https://stackoverflow.com/a/18283757 ) libstdc++ is built
 with the {{{__USE_MINGW_ANSI_STDIO}}} define, so calls to the printf and
 wprintf function families will use the expected format string behaviour
 found on non-windows platforms. Apparently libstdc++ depends on the ANSI
 stdio behavior so it's toggled on.

 The following program will output differently depending on which headers
 you use (C or C++) even if you build with mingw g++
 {{{
 #ifdef USE_STDC
 #include <stdio.h>
 #include <wchar.h>
 #else
 #include <cstdio>
 #include <cwchar>
 ##endif

 int main()
 {
     printf("1: %s\n", "ansi format, ansi string");
     printf("2: %S\n", L"ansi format, wide string");
     wprintf(L"3: %s\n", "wide format, ansi string");
     wprintf(L"4: %S\n", L"wide format, wide string");
     fflush(stdout);
 }
 }}}


 ----


 If you build with the C headers, you get the following output (wine on
 linux):

 {{{
 1: ansi format, ansi string
 2: ansi format, wide string
 3: 4: w
 }}}

 This it the output you would expect if you built with vcs given
 Microsoft's printf format specifiers ( https://docs.microsoft.com/en-
 us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-
 functions?view=vs-2019 )

 The first two prints work as expected, but the next two fail:
  3 is expecting the string to be a wide character string, but it
 encounters an ansi string. I don't know what this ansi string encodes as
 wide string, but it would seem wprintf gives up trying to print it. For
 shits and giggles, if we replace 3's ansi string with
 {{{"w\0i\0d\0e\0\0"}}} it prints out 'wide' to the console.
  4 is expecting an ansi string but gets a wide string, so it prints the
 ansi 'w', finds a NULL byte and treats it as a terminator, only printing
 out 'w'.

 ----

 If you build with the C++ headers (or if you build with the C headers and
 add {{{#define __USE_MINGW_ANSI_STDIO 1}}} before including {{{stdio.h}}},
 you get the following output:
 {{{
 1: ansi format, ansi string
 2: ansi format, wide string
 3: wide format, ansi string
 4: wide format, wide string
 }}}

 Which is as one would expect I think. Undefining
 {{{__USE_MINGW_ANSI_STDIO}}} or redifining it to 0 does not seem to
 reverse the effect when  using the C++ headers.

 ----

 However, we probably shouldn't go fucking around with this define because
 who what else depends on this behaviour to work. Instead, we should
 replace the _vsnwprintf call with StringCchVPrintfW. As StringCchVPrintfW
 has no expectation of conforming to ANSI C string format specifiers, it
 can instead ignore the {{{__USE_MINGW_ANSI_STDIO}}} define and always
 conform the Microsoft's specifiers. The following example builds and runs
 using the Microsoft formats:

 {{{
 #include <windows.h>
 #include <strsafe.h>

 #include <iostream>
 using namespace std;

 int main()
 {
     char abuffer[1024];
     wchar_t wbuffer[1024];

     StringCchPrintfA(abuffer, 1024, "1: %s\n", "ansi format, ansi
 string"); // small S for ansi
     cout << abuffer;
     StringCchPrintfA(abuffer, 1024, "2: %S\n", L"ansi format, wide
 string"); // big S for wide
     cout << abuffer;

     cout << flush;

     StringCchPrintfW(wbuffer, 1024, L"3 : %S\n", "wide format, ansi
 string"); // big S for ansi
     wcout << wbuffer;
     StringCchPrintfW(wbuffer, 1024, L"4': %s\n", L"wide format, wide
 string"); // small S for wide
     wcout << wbuffer;

     wcout << flush;
 }
 }}}

 ----

 This patch replaces the _vsnwprintf with StringCchVPrintfExW from
 strsafe.h, a windows specific function and as such implements Microsoft's
 format-string specifiers. The new implementation of mywcsprintf has the
 same outputs and retval as the old for a handful of test-cases.

 **tor-browser patch**:

 https://gitweb.torproject.org/user/richard/tor-
 browser.git/commit/?h=bug_31567

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31567#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online