[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-bugs] #31567 [Applications/Tor Browser]: NS_tsnprintf() does not handle %s correctly on Windows
#31567: NS_tsnprintf() does not handle %s correctly on Windows
-------------------------------------+-------------------------------------
Reporter: mcs | Owner: gk
Type: defect | Status: assigned
Priority: Very High | Milestone:
Component: Applications/Tor | Version:
Browser | Keywords: ff68-esr, tbb-9.0-must-
Severity: Critical | alpha, TorBrowserTeam201908
Actual Points: | Parent ID:
Points: | Reviewer:
Sponsor: |
-------------------------------------+-------------------------------------
While testing the ESR68-based updater on Windows, Kathy and I found a
Windows toolchain problem. We tested using our own 64-bit build (rbm
nightly build process) on a Windows 10 system.
We discovered that file paths generated using format strings are
corrupted. Specifically, calls to `NS_tsnprintf()` do not work correctly;
apparently, because that is a "wide" function a `%s` when used in the
format string is supposed to mean "expect the arg to be of type WCHAR *".
Instead, the args are processed as C-style strings which means they get
truncated after the first character (at least when using characters that
fit within the first 256 Unicode codepoints).
`NS_tsnprintf()` is a macro that is defined in
toolkit/mozapps/update/common/updatedefines.h (all of the code that uses
it is related to the updater). We first noticed that problem when we saw a
failure inside updater.cpp's `WriteToFile()` function. The following code
from that function fails because it tries to move `filename` to a bad
path.
{{{
#if defined(XP_WIN)
NS_tchar dstfilename[MAXPATHLEN] = {NS_T('\0')};
NS_tsnprintf(dstfilename, sizeof(dstfilename) / sizeof(dstfilename[0]),
NS_T("%s\\%s"), gPatchDirPath, aFilename);
if (MoveFileExW(filename, dstfilename, MOVEFILE_REPLACE_EXISTING) == 0)
{
return false;
}
#endif
}}}
The computed path is `C\u` (the first character from `gPatchDirPath`
followed by the \ and then the first character of `aFilename`).
On Windows, `NS_tsnprintf()` is defined as `mywcsprintf` which is an
inline function that uses `_vsnwprintf()`. We have not traced this bug
deeper than that point, but we did verify that the problems disappear if
we replace all occurrences of `%s` with `%S` in the `NS_tsnprintf()`
format strings. That is a very ugly fix though, and it would be wrong on
macOS and Linux.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31567>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs