Skip to content
GitLab
  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • Trac Trac
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Issues 246
    • Issues 246
    • List
    • Boards
    • Service Desk
    • Milestones
  • Monitor
    • Monitor
    • Metrics
    • Incidents
  • Analytics
    • Analytics
    • Value stream
  • Wiki
    • Wiki
  • Activity
  • Create a new issue
  • Issue Boards
Collapse sidebar
  • Legacy
  • TracTrac
  • Issues
  • #31567
Closed (moved) (moved)
Open
Created Aug 30, 2019 by Mark Smith@mcs

NS_tsnprintf() does not handle %s correctly on Windows

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.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking