memory corruption / security bug in sysread,syswrite + patch

Using a negative offset in sysread() gives interesting results.
I get either

	assertion botched: OV_MAGIC(op, bucket) == MAGIC
	zsh: 22828 abort      perl xp1 < /etc/passwd

if the offset is 'mild' or

	panic: realloc at xp2 line 1.

if the offset is 'wild'.

Using a negative offset in syswrite() opens up interesting vistas,
like, say, your stack :-)

A patch follows. 'Reasonably' small negative values are accepted,
they count from the end of the data. One possible point of debate:
should that be +1? That is, if the offset is negative, should that
mean 1) at the point 2) after the point? For sysread(), "after" might
sometimes be a better choice?

p5p-msgid: <199611231705.TAA02671@alpha.hut.fi>
private-msgid: <199611251946.VAA30459@alpha.hut.fi>

2 files changed, 21 insertions, 8 deletions

diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index d4998cf44b..1c063a6240 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -3003,9 +3003,10 @@ Attempts to read LENGTH bytes of data into variable SCALAR from the
 specified FILEHANDLE, using the system call read(2).  It bypasses
 stdio, so mixing this with other kinds of reads may cause confusion.
 Returns the number of bytes actually read, or undef if there was an
-error.  SCALAR will be grown or shrunk to the length actually read.  An
-OFFSET may be specified to place the read data at some other place than
-the beginning of the string.
+error.  SCALAR will be grown or shrunk to the length actually read.
+An OFFSET may be specified to place the read data at some other
+place than the beginning of the string. A negative OFFSET counts
+backwards from the end of the string.
 
 =item system LIST
 
@@ -3025,9 +3026,10 @@ described in L<perlop/"`STRING`">.
 Attempts to write LENGTH bytes of data from variable SCALAR to the
 specified FILEHANDLE, using the system call write(2).  It bypasses
 stdio, so mixing this with prints may cause confusion.  Returns the
-number of bytes actually written, or undef if there was an error.  An
-OFFSET may be specified to get the write data from some other place than
-the beginning of the string.
+number of bytes actually written, or undef if there was an error.
+An OFFSET may be specified to get the write data from some other place
+than the beginning of the string. A negative OFFSET counts backwards
+from the end of the string.
 
 =item tell FILEHANDLE
 
diff --git a/pp_sys.c b/pp_sys.c
index 40a0d3512c..8928e89c8c 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -1087,6 +1087,7 @@ PP(pp_sysread)
     if (op->op_type == OP_RECV) {
 	bufsize = sizeof buf;
 	buffer = SvGROW(bufsv, length+1);
+	/* 'offset' means 'flags' here */
 	length = recvfrom(PerlIO_fileno(IoIFP(io)), buffer, length, offset,
 	    (struct sockaddr *)buf, &bufsize);
 	if (length < 0)
@@ -1107,6 +1108,11 @@ PP(pp_sysread)
     if (op->op_type == OP_RECV)
 	DIE(no_sock_func, "recv");
 #endif
+    if (offset < 0) {
+        if (-offset > blen)
+	   DIE("Too negative offset");
+	offset += blen;
+    }
     bufsize = SvCUR(bufsv);
     buffer = SvGROW(bufsv, length+offset+1);
     if (offset > bufsize) { /* Zero any newly allocated space */
@@ -1179,9 +1185,14 @@ PP(pp_send)
 	}
     }
     else if (op->op_type == OP_SYSWRITE) {
-	if (MARK < SP)
+        if (MARK < SP) {
 	    offset = SvIVx(*++MARK);
-	else
+	    if (offset < 0) {
+	        if (-offset > blen)
+		    DIE("Too negative offset");
+		offset += blen;
+	    }
+	} else
 	    offset = 0;
 	if (length > blen - offset)
 	    length = blen - offset;