[Mono-bugs] [Bug 375580] New: Buffer overrun plus infinite loop on partial write to serial port - patch included

[bugzilla_noreply at novell.com]

https://bugzilla.novell.com/show_bug.cgi?id=375580


           Summary: Buffer overrun plus infinite loop on partial write to
                    serial port - patch included
           Product: Mono: Class Libraries
           Version: 1.2.6
          Platform: i386
        OS/Version: Linux
            Status: NEW
          Severity: Normal
          Priority: P5 - None
         Component: Mono.POSIX
        AssignedTo: miguel at novell.com
        ReportedBy: avillaci at ceibo.fiec.espol.edu.ec
         QAContact: mono-bugs at lists.ximian.com
          Found By: Customer


Created an attachment (id=205123)
 --> (https://bugzilla.novell.com/attachment.cgi?id=205123)
Fix buffer overrun and endless loop on partial write

This patch fixes two bugs in the write_serial() function in the Mono POSIX
helper library. 

>From what I understand of the Mono sources, managed code in SerialPortStream
checks constraints on the data to be sent to the serial port, and then calls
the following function in the POSIX helper:

int write_serial (int fd, guchar *buffer, int offset, int count, int timeout)

The 'offset' parameter is supposed to contain the byte offset from which the
data to be written starts (relative to 'buffer'), and 'count' is supposed to
be the number of bytes to write. 

The first bug is at line 98 of support/serial.c:

> 	n = count - offset;

This subtraction is, IMHO, pointless in this function. Later in the function, 
'n' is treated like a count of remaining bytes to send, but this subtraction
treats 'count' as the length of the entire buffer, which is inconsistent with 
later treatement of the buffer. The subtraction is a noop when offset is 0, but
breaks when offset > count (for example, sending 20 bytes from offset 40 of a 
100-byte buffer), which is allowed by the checks in managed code. Since 'n'
is of type guint32, the subtraction results in a very large unsigned integer,
which immediately causes a buffer overflow when writing to the serial port.

The second bug is at lie 116 of support/serial.c:

> 		t = write(fd, buffer + offset, count);

In case where write() informs of a partial read (when t > 0 && t < count), this
code is incorrect, because 'count' is invariant inside the write() loop. As 
evidenced at line 127, 'n' is decremented by the amount reported by write(), so
'n' is intended to hold the amount of remaining bytes to write. In case of a 
short write, the write is retried with 'count' bytes as before, but with a 
modified 'offset'. Depending of the values of 'offset' and 'count', this will
eventually write data past the end of 'buffer' to the serial port. In
particular, with 'offset' = 0 and 'count' equal to the full length of 'buffer',
the second write after the first short write WILL read past the end of
'buffer'. Meanwhile, 'n' keeps being decremented. Since the second read uses
'count' > 'n', a full write (or a very big short write) will probably cause 't'
to be greater than 'n'. 
Unless the writes happen to reduce 'n' exactly to 0, the subtraction eventually 
wraps around (since guint32 is unsigned) into a very large positive integer, 
which results in an endless loop of writes of data past the end of buffer, and 
an eventual program crash.

This patch fixes both issues by removing the subtraction of 'offset' and
instead
assigning 'n' to the value of 'count' directly, and by using 'n' instead of 
'count' as the count of remaining bytes to write, as intended.

This patch applies against mono 1.2.6 .


-- 
Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.