From: Jarkko Hietaniemi Date: Mon, 25 Nov 1996 19:46:31 +0000 (+0200) Subject: memory corruption / security bug in sysread,syswrite + patch X-Git-Tag: accepted/trunk/20130322.191538~38041^2~626^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d7090df90a9cb89c83787d916e40d92a616b146d;p=platform%2Fupstream%2Fperl.git 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> --- diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod index d4998cf..1c063a6 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. 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 40a0d35..8928e89 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;