GitLab

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.