Null termination of VERSIONINFO strings?

[BenjaminRi]

Track issue mxre/winres#25

[BenjaminRi]

I'm doing some research on this issue:

https://devblogs.microsoft.com/oldnewthing/20040130-00/?p=40813

The strings in each bundle are stored as counted UNICODE strings, not null-terminated strings.

Note that we must explicitly null-terminate the string since the string in the resource is not null-terminated.

https://stackoverflow.com/questions/45491778/why-do-mfc-rc-files-sometimes-have-a-manually-inserted-0-at-the-end
https://stackoverflow.com/questions/7745076/vc-resource-compiler-rc-option-n

LoadString() (or more exactly LoadStringW() as ANSI variant cannot do that) can be used to retrieve the raw string resource when you specify 0 as the buffer size:

WCHAR* str;
int str_len;
str_len = LoadStringW(hInstance, ID_STRING, (LPWSTR) &str, 0);

In this case it just stores the address of the original string resource into the str as mapped into process memory and no string copying happens. Obviously the LoadLibrary() implementation then simply cannot add the terminator and this is when the resource compiler option is handy as work with zero-terminated strings is so much easier then using the string length (the return value of LoadLibrary()).

-> Long story short, it looks like, indeed, we should NULL-terminate them

[BenjaminRi]

https://devblogs.microsoft.com/oldnewthing/20061221-02/?p=28643

Hmmm... It looks like it already adds a NULL somehow? The length is 0c 00 (so, 12). The string is example.exe, which is 11 characters, so it does add a NULL.

0010c5e0  61 00 72 00 00 00 00 00  40 00 0c 00 01 00 4f 00  |a.r.....@.....O.|
0010c5f0  72 00 69 00 67 00 69 00  6e 00 61 00 6c 00 46 00  |r.i.g.i.n.a.l.F.|
0010c600  69 00 6c 00 65 00 6e 00  61 00 6d 00 65 00 00 00  |i.l.e.n.a.m.e...|
0010c610  65 00 78 00 61 00 6d 00  70 00 6c 00 65 00 2e 00  |e.x.a.m.p.l.e...|
0010c620  65 00 78 00 65 00 00 00  30 00 06 00 01 00 50 00  |e.x.e...0.....P.|

The only thing I don't understand is why in the blog post, the length seems to be in bytes, but here it is in characters.

[BenjaminRi]

Would you look at that, Mr. Chen has an answer for this as well. Not surprised at all.

https://devblogs.microsoft.com/oldnewthing/20061222-00/?p=28623

A common mistake in generating 32-bit resources is to mistreat the cbData field of the structure I called a VERSIONNODE as a count of characters rather than a count of bytes if the type is Unicode text. Even Microsoft’s own Resource Compiler has fallen into this trap! For example, consider this VERSIONNODE I presented last time:

These malformed version resources manage to get away without crashing too horribly because the standard format of version resources uses string data only in leaf nodes. Therefore, the incorrect cbData affects only the node itself and doesn’t cause the child nodes to be parsed incorrectly (since there are no child nodes).

Uhh... So, that means my resource generator is bugged?

[BenjaminRi]

Yep, the wrong number of bytes looks like a binutils bug.

https://sourceware.org/git/?p=binutils-gdb.git;a=blob_plain;f=binutils/resbin.c;hb=HEAD

/* Convert a stringtable resource to binary.  */

static rc_uint_type
res_to_bin_stringtable (windres_bfd *wrbfd, rc_uint_type off,
			const rc_stringtable *st)
{
  int i;

  for (i = 0; i < 16; i++)
    {
      rc_uint_type slen, length;
      unichar *s;

      slen = (rc_uint_type) st->strings[i].length;
      if (slen == 0xffffffff) slen = 0;
      s = st->strings[i].string;

      length = 2 + slen * 2;
      if (wrbfd)
	{
	  bfd_byte *hp;
	  rc_uint_type j;

	  hp = (bfd_byte *) reswr_alloc (length);
	  windres_put_16 (wrbfd, hp, slen);

      for (j = 0; j < slen; j++)
	    windres_put_16 (wrbfd, hp + 2 + j * 2, s[j]);
	  set_windres_bfd_content (wrbfd, hp, off, length);
    }
      off += length;
    }
  return off;
}

As we can see here windres_put_16 (wrbfd, hp, slen);, they write the number of characters instead of the number of bytes, which would be slen*2.

The opposite direction is also wrong but it does have a length check:

/* Convert a stringtable resource from binary.  */

static rc_res_resource *
bin_to_res_string (windres_bfd *wrbfd, const bfd_byte *data, rc_uint_type length)
{
  rc_stringtable *st;
  int i;
  rc_res_resource *r;

  st = (rc_stringtable *) res_alloc (sizeof (rc_stringtable));

  for (i = 0; i < 16; i++)
    {
      unsigned int slen;

      if (length < 2)
	toosmall (_("stringtable string length"));
      slen = windres_get_16 (wrbfd, data, 2);
      st->strings[i].length = slen;

      if (slen > 0)
	{
	  unichar *s;
	  unsigned int j;

	  if (length < 2 + 2 * slen)
	    toosmall (_("stringtable string"));

	  s = (unichar *) res_alloc (slen * sizeof (unichar));
	  st->strings[i].string = s;

	  for (j = 0; j < slen; j++)
	    s[j] = windres_get_16 (wrbfd, data + 2 + j * 2, 2);
	}

      data += 2 + 2 * slen;
      length -= 2 + 2 * slen;
    }

  r = (rc_res_resource *) res_alloc (sizeof *r);
  r->type = RES_TYPE_STRINGTABLE;
  r->u.stringtable = st;

  return r;
}