csvsj@garnet.berkeley.edu (Steve Jacobson) (06/30/89)
There is a bug in the XwStringSourceCheckData() function in SourceStr.c
that causes data to be written past the bounds of a memory chunk allocated
by XtMalloc(). This situation occurs when:
1. XtNsource is the default StringSrc
2. an initial string is specified in the XtNstring resource
3. a maximum buffer size equal to the length of the initial string is
specified in the XtNmaximumsize resource
As with most memory overwrite bugs, the effect of the bug depends on the
specific memery layout at the time it occurs. The effects are seen (if they
are seen) at a later time when the overwritten memory is referenced. In
our application, the bug led to a segmentation violation in a subsequent
malloc() call. This occurred on our Mac II/AUX, MicroVax, DEC 3100, and
IBM-RT platforms. No noticable effects occured on our SUN 3 platform.
The bug overwrites only one byte; someone has suggested to me that the
SUN malloc allocates more bytes than the call requested, thereby preventing
the bug from affecting the program operation.
Here are the details:
In XwStringSourceCheckData(), the code lines:
if (data->max_size_flag)
data->buffer_size = data->max_size;
make the data buffer size equal to the maximumSize resource when one is
specified.
The code line:
initial_size = XwStrlen(data->initial_string);
makes the initial size equal to the length of the string resource when one is
specified.
The code lines:
if (data->buffer && initial_size)
data->buffer =
(unsigned char *) XtRealloc(data->buffer, data->buffer_size);
else if (!data->buffer)
data->buffer = (unsigned char *) XtMalloc(data->buffer_size);
allocate data->buffer_size bytes of memory.
The code lines:
if (initial_size) {
strcpy(data->buffer, data->initial_string);
data->initial_string = NULL;
}
will write data->buffer_size + 1 bytes when the length of the initial string
is equal to data->buffer_size. The concluding null is written past the
last byte allocated in the previous code.
Suggested Fix:
Since it is clear that the widget uses its own state variables to keep
track of string length related information, it is unnecessary to include
the concluding null in the data buffer, although it does no harm if there
is room for it in the buffer. Replacing the strcpy() line with:
strncpy(data->buffer, data->initial_string, data->buffer_size);
insures that memory will not be overwritten. We made this change and our
segmentation violations went away.
Alternatively, an extra byte could be requested in the memory allocation
code lines; that is a more cautious approach.