[Bug libc/3013] New: Uninitialized byte written in fa_IR/LC_CTYPE

[Bug libc/3013] New: Uninitialized byte written in fa_IR/LC_CTYPE

[barbier at linuxfr dot org]

Hi,

locale/programs/ld-ctype.c (ctype_output) contains several occurences of
  iov[2 + elem + offset].iov_base = (void *) nulbytes;
  iov[2 + elem + offset].iov_len = 1 + (4 - ((total + 1) % 4));

The last expression is between 2 and 5, but nulbytes is defined by
  static const char nulbytes[4] = { 0, 0, 0, 0 };
An uninitialized byte can thus be written in LC_CTYPE, this happens
with fa_IR.

-- 
           Summary: Uninitialized byte written in fa_IR/LC_CTYPE
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: barbier at linuxfr dot org
                CC: glibc-bugs at sources dot redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/3013] Uninitialized byte written in fa_IR/LC_CTYPE

[rsandifo at sourceware dot org]

------- Additional Comments From rsandifo at sourceware dot org  2006-08-09 14:58 -------
FWIW, we've been using the first of the attached patches to fix this issue,
and to fix a couple of other problems we'd found in the localedef code.

There are two occurences of:

    iov[2 + elem + offset].iov_base = (void *) nulbytes;
    iov[2 + elem + offset].iov_len = 1 + (4 - ((total + 1) % 4));
    total += 1 + (4 - ((total + 1) % 4));

in ld-ctype.c, and like you say, it adds between 2 and 5 bytes.

This is a rather odd construct.  The data being added here is the nul
terminator for a list of strings.  I think the idea is to just to add
one nul and then align the following data to 4 bytes.  (The nul
terminator of the final string has already been added; the nul being
added here terminates the list itself.)  However, if we add a nul
terminator at file offset 4X + 3, the calculation:

   (4 - ((total + 1) % 4))

means that we'll insert 4 bytes of padding after the nul.  So what we
effectively have is two nul terminators followed by an alignment.

In theory it should be safe to remove the extra nul without affecting
readers of LC_CTYPE, but there might not be any appetite for changing
the format now.  I've therefore attached two patches.  The first patch
keeps the original padding and expands nulbytes[] accordingly.  The
second patch changes the padding calculation instead.

The other problems we'd found, and that are fixed by both patches, were:

- In the same function, the code to add DEFAULT_MISSING wasn't
  setting iov_len correctly:

       iov[2 + elem + offset].iov_base =
	 ctype->default_missing ?: (uint32_t *) L"";
       iov[2 + elem + offset].iov_len =
	 wcslen (iov[2 + elem + offset].iov_base);
       idx[elem + 1] = idx[elem] + iov[2 + elem + offset].iov_len;

  wcslen() returns a character count, not a byte count, so the iov_len
  value must be multiplied by sizeof (uint32_t).  This showed up for a
  single-character DEFAULT_MISSING on a big-endian target.

- The same function builds up class_b[] as follows:

     for (nr = 0; nr < ctype->nr_charclass; nr++)
       {
	 ctype->class_b[nr] = (uint32_t *) xcalloc (256 / 32, sizeof (uint32_t));

	 for (idx = 0; idx < 256; ++idx)
	   if (ctype->class256_collection[idx] & _ISbit (nr))
	     ctype->class_b[nr][idx >> 5] |= (uint32_t)1 << (idx & 0x1f);
       }

  The problem here is that _ISbit() is only designed to handle
  arguments in the range [0,11] and nr_charclass can be higher
  than that.   The code deals with this correctly when setting
  class256_collection -- for example:

	 class256_bit = cnt <= 11 ? _ISbit (cnt) : 0;

  -- but not when reading it.

  This causes problems for little-endian machines, for which
  _ISbit() is defined as follows:

     #define _ISbit(bit) \
       ((bit) < 8 ? ((1 << (bit)) << 8) : ((1 << (bit)) >> 8))

  nr_charclass == 19 (IIRC) for the Japenese files, and this definition
  of _ISbit() treats bits [16,18] like bits [0,2].

- time_output was failing to align _NL_W_DATE_FMT.

Both patches tested on x86_64-linux-gnu.

Richard


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Uninitialized byte written  |Uninitialized byte written
                   |in fa_IR/LC_CTYPE           |in fa_IR/LC_CTYPE


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/3013] Uninitialized byte written in fa_IR/LC_CTYPE

[rsandifo at sourceware dot org]

------- Additional Comments From rsandifo at sourceware dot org  2006-08-09 14:59 -------
Created an attachment (id=1216)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=1216&action=view)
Patch that changes the padding


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/3013] Uninitialized byte written in fa_IR/LC_CTYPE

[rsandifo at sourceware dot org]

------- Additional Comments From rsandifo at sourceware dot org  2006-08-09 14:58 -------
Created an attachment (id=1215)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=1215&action=view)
Patch that keeps the existing padding


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/3013] Uninitialized byte written in fa_IR/LC_CTYPE

[barbier at linuxfr dot org]

------- Additional Comments From barbier at linuxfr dot org  2006-08-09 20:51 -------
Thanks for your feedback, Richard.  In your 2nd patch, you should
not modify the size of nulbytes, and maybe also replace 'total'
computations by
     total += iov[2 + elem + offset].iov_len;
which is less error prone.

Out of curiosity, did you also patch ld-collate.c?  There are
some differences between architectures but I did not investigate
yet.


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/3013] Uninitialized byte written in fa_IR/LC_CTYPE

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-08-12 20:19 -------
I applied a slightly changed patch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=3013

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.