Buffer overflow in win32_select() (PATCH included) #9057

p5pRT · 2007-10-10T09:44:12Z

Migrated from rt.perl.org#46309 (status was 'resolved')

Searchable as RT46309$

p5pRT · 2007-10-10T09:44:13Z

From risto.kankkunen@f-secure.com

This is a bug report for perl from risto.kankkunen@f-secure.com,
generated with the help of perlbug 1.35 running under perl v5.8.7.

I've found a nasty buffer overflow problem in win32_select() function
that has made our product, which is using embedded Perl, very
unreliable. We are currently using version 5.8.7, but I have checked
that the problematic code is the same in 5.8.8.

The problem happens, when some of the select() sets are not specified.
In this case the code uses a dummy variable in place of the actual bit
vector:

win32/win32sck.c:282:

int
win32_select(int nfds, Perl_fd_set* rd, Perl_fd_set* wr, Perl_fd_set* ex,
const struct timeval* timeout)
{
int r;
#ifdef USE_SOCKETS_AS_HANDLES
Perl_fd_set dummy;

win32/win32sck.c:304:

if \(\!rd\)
rd = &dummy\, prd = NULL;

Perl_fd_set is defined in win32/include/sys/socket.h:26 as:

#ifndef PERL_FD_SETSIZE
#define PERL_FD_SETSIZE 64
#endif

#define PERL_BITS_PER_BYTE 8
#define PERL_NFDBITS (sizeof(Perl_fd_mask)*PERL_BITS_PER_BYTE)

typedef int Perl_fd_mask;

typedef struct Perl_fd_set {
Perl_fd_mask bits[(PERL_FD_SETSIZE+PERL_NFDBITS-1)/PERL_NFDBITS];
} Perl_fd_set;

If a socket number exceeds 63, this code writes over the end of dummy,
destroying some local variables, the return address and so on:

win32/win32sck.c:334:

for \(i = 0; i \< nfds; i\+\+\) \{
fd = TO\_SOCKET\(i\);
if \(PERL\_FD\_ISSET\(i\,rd\) && \!FD\_ISSET\(fd\, &nrd\)\)
    PERL\_FD\_CLR\(i\,rd\);

You can easily repro the problem with the attached socketcrash.pl script.

I have attached a patch that fixes the buffer overflow by eliminating
the dummy
variable.

Regards,
Risto Kankkunen

use strict;
use warnings;

use IO&#8203;::Socket;

sub fhbits \{
&nbsp;    my @&#8203;fhlist = @&#8203;\_;

&nbsp;    my\($bits\)="";
&nbsp;    vec\($bits\,fileno\($\_\)\,1\) = 1 for \(@&#8203;fhlist\);
&nbsp;    return $bits;
\}

my @&#8203;handles = \(\);
&nbsp;    for \(1\.\.2000\) \{
&nbsp;    my $handle = new IO&#8203;::Socket\(
&nbsp;        Domain=>AF\_INET\,
&nbsp;        Proto=>"udp"\,
&nbsp;        PeerAddr=>"127\.0\.0\.2&#8203;:8888"
&nbsp;    \) or die;
&nbsp;    
&nbsp;    push\(@&#8203;handles\, $handle\);
\}
printf "handles=%d\-%d\\n"\, fileno\($handles\[0\]\)\, fileno\($handles\[\-1\]\);

my $rin = fhbits\(@&#8203;handles\);
my \($rout\, $wout\, $eout\);

my \($count\, $timeleft\) = select\($rout=$rin\, $wout=undef\, $eout=undef\, 1\);

print "count=$count\\n";
print "timeleft=$timeleft\\n";
printf "rin =%\*v08b\\n"\, " " \, $rin;
printf "rout=%\*v08b\\n"\, " " \, $rout || 0;
printf "wout=%\*v08b\\n"\, " " \, $wout || 0;
printf "eout=%\*v08b\\n"\, " " \, $eout || 0;
~~~ win32\_select\.patch ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Index&#8203;: perl/src/win32/win32sck\.c
===================================================================
\-\-\- perl/src/win32/win32sck\.c    \(revision 3794\)
\+\+\+ perl/src/win32/win32sck\.c    \(working copy\)
@&#8203;@&#8203; \-287\,9 \+287\,8 @&#8203;@&#8203;
&nbsp;  \{
&nbsp;      int r;
&nbsp;  \#ifdef USE\_SOCKETS\_AS\_HANDLES
\-    Perl\_fd\_set dummy;
&nbsp;      int i\, fd\, save\_errno = errno;
\-    FD\_SET nrd\, nwr\, nex\, \*prd\, \*pwr\, \*pex;
\+    FD\_SET nrd\, nwr\, nex;

&nbsp;      /\* winsock seems incapable of dealing with all three null fd\_sets\,
&nbsp;       \* so do the \(millisecond\) sleep as a special case
@&#8203;@&#8203; \-303\,44 \+302\,31 @&#8203;@&#8203;
&nbsp;      return 0;
&nbsp;      \}
&nbsp;      StartSockets\(\);
\-    PERL\_FD\_ZERO\(&dummy\);
\-    if \(\!rd\)
\-    rd = &dummy\, prd = NULL;
\-    else
\-    prd = &nrd;
\-    if \(\!wr\)
\-    wr = &dummy\, pwr = NULL;
\-    else
\-    pwr = &nwr;
\-    if \(\!ex\)
\-    ex = &dummy\, pex = NULL;
\-    else
\-    pex = &nex;

&nbsp;      FD\_ZERO\(&nrd\);
&nbsp;      FD\_ZERO\(&nwr\);
&nbsp;      FD\_ZERO\(&nex\);
&nbsp;      for \(i = 0; i \< nfds; i\+\+\) \{
&nbsp;      fd = TO\_SOCKET\(i\);
\-    if \(PERL\_FD\_ISSET\(i\,rd\)\)
\+    if \(rd && PERL\_FD\_ISSET\(i\,rd\)\)
&nbsp;          FD\_SET\(\(unsigned\)fd\, &nrd\);
\-    if \(PERL\_FD\_ISSET\(i\,wr\)\)
\+    if \(wr && PERL\_FD\_ISSET\(i\,wr\)\)
&nbsp;          FD\_SET\(\(unsigned\)fd\, &nwr\);
\-    if \(PERL\_FD\_ISSET\(i\,ex\)\)
\+    if \(ex && PERL\_FD\_ISSET\(i\,ex\)\)
&nbsp;          FD\_SET\(\(unsigned\)fd\, &nex\);
&nbsp;      \}

&nbsp;      errno = save\_errno;
\-    SOCKET\_TEST\_ERROR\(r = select\(nfds\, prd\, pwr\, pex\, timeout\)\);
\+    SOCKET\_TEST\_ERROR\(r = select\(nfds\, &nrd\, &nwr\, &nex\, timeout\)\);
&nbsp;      save\_errno = errno;

&nbsp;      for \(i = 0; i \< nfds; i\+\+\) \{
&nbsp;      fd = TO\_SOCKET\(i\);
\-    if \(PERL\_FD\_ISSET\(i\,rd\) && \!FD\_ISSET\(fd\, &nrd\)\)
\+    if \(rd && PERL\_FD\_ISSET\(i\,rd\) && \!FD\_ISSET\(fd\, &nrd\)\)
&nbsp;          PERL\_FD\_CLR\(i\,rd\);
\-    if \(PERL\_FD\_ISSET\(i\,wr\) && \!FD\_ISSET\(fd\, &nwr\)\)
\+    if \(wr && PERL\_FD\_ISSET\(i\,wr\) && \!FD\_ISSET\(fd\, &nwr\)\)
&nbsp;          PERL\_FD\_CLR\(i\,wr\);
\-    if \(PERL\_FD\_ISSET\(i\,ex\) && \!FD\_ISSET\(fd\, &nex\)\)
\+    if \(ex && PERL\_FD\_ISSET\(i\,ex\) && \!FD\_ISSET\(fd\, &nex\)\)
&nbsp;          PERL\_FD\_CLR\(i\,ex\);
&nbsp;      \}
&nbsp;      errno = save\_errno;

Flags:
category=core
severity=high

Site configuration information for perl v5.8.7:

Configured by kankri at Mon Oct 30 15:14:16 2006.

Summary of my perl5 (revision 5 version 8 subversion 7) configuration:
Platform:
osname=MSWin32, osvers=5.1, archname=MSWin32-x86-multi-thread
uname=''
config_args='undef'
hint=recommended, useposix=true, d_sigaction=undef
usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
use64bitint=undef use64bitall=undef uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cl', ccflags ='-nologo -Gf -W3 -MD -Zi -DNDEBUG -O1 -DWIN32
-D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DPERL_NO_EXIT
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
-DPERL_MSVCRT_READFIX',
optimize='-MD -Zi -DNDEBUG -O1',
cppflags='-DWIN32'
ccversion='12.00.8804', gccversion='', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"c:\fs-perl\lib\CORE" -machine:x86'
libpth=\lib
libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib
odbc32.lib odbccp32.lib msvcrt.lib
perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib msvcrt.lib
libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=fspl58.lib
gnulibc_version='undef'
Dynamic Linking:
dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -libpath:"c:\fs-perl\lib\CORE" -machine:x86'

Locally applied patches:

@INC for perl v5.8.7:
C:/fs-perl/lib
C:/fs-perl/site/lib
.

Environment for perl v5.8.7:
HOME (unset)
LANG (unset)
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=<undisclosed>
PERL_BADLANG (unset)
SHELL (unset)

p5pRT · 2007-10-19T10:26:14Z

From @rgs

On 10/10/2007, via RT Risto Kankkunen <perlbug-followup@perl.org> wrote:

I've found a nasty buffer overflow problem in win32_select() function
that has made our product, which is using embedded Perl, very
unreliable. We are currently using version 5.8.7, but I have checked
that the problematic code is the same in 5.8.8.

No feedback on this from our Win32 experts ?

p5pRT · 2007-10-19T10:26:16Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2007-11-08T14:24:19Z

From @steve-m-hay

Rafael Garcia-Suarez wrote:

On 10/10/2007, via RT Risto Kankkunen <perlbug-followup@perl.org>
wrote:

I've found a nasty buffer overflow problem in win32_select() function
that has made our product, which is using embedded Perl, very
unreliable. We are currently using version 5.8.7, but I have checked
that the problematic code is the same in 5.8.8.

No feedback on this from our Win32 experts ?

I can confirm that the bug still exists in bleadperl (@32245) and that
the patch in this bug report fixes it (and the test suite still passes
all tests).

The patch *looks* OK to me, but I don't really know. Anyone else care to
comment on it? Is it too late for 5.10 now anyway?

p5pRT · 2008-06-17T07:24:52Z

From @rgs

2007/11/8 Steve Hay <SteveHay@planit.com>:

Rafael Garcia-Suarez wrote:

On 10/10/2007, via RT Risto Kankkunen <perlbug-followup@perl.org>
wrote:

I've found a nasty buffer overflow problem in win32_select() function
that has made our product, which is using embedded Perl, very
unreliable. We are currently using version 5.8.7, but I have checked
that the problematic code is the same in 5.8.8.

No feedback on this from our Win32 experts ?

I can confirm that the bug still exists in bleadperl (@32245) and that
the patch in this bug report fixes it (and the test suite still passes
all tests).

The patch *looks* OK to me, but I don't really know. Anyone else care to
comment on it? Is it too late for 5.10 now anyway?

I finally applied it as #34067 to blead.

p5pRT · 2008-06-17T07:24:56Z

@rgs - Status changed from 'open' to 'resolved'

p5pRT closed this as completed Jun 17, 2008

p5pRT added Severity High distro-mswin32 type-core labels Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Buffer overflow in win32_select() (PATCH included) #9057

Buffer overflow in win32_select() (PATCH included) #9057

p5pRT commented Oct 10, 2007

p5pRT commented Oct 10, 2007

p5pRT commented Oct 19, 2007

p5pRT commented Oct 19, 2007

p5pRT commented Nov 8, 2007

p5pRT commented Jun 17, 2008

p5pRT commented Jun 17, 2008

Buffer overflow in win32_select() (PATCH included) #9057

Buffer overflow in win32_select() (PATCH included) #9057

Comments

p5pRT commented Oct 10, 2007

p5pRT commented Oct 10, 2007

From risto.kankkunen@f-secure.com

p5pRT commented Oct 19, 2007

From @rgs

p5pRT commented Oct 19, 2007

p5pRT commented Nov 8, 2007

From @steve-m-hay

p5pRT commented Jun 17, 2008

From @rgs

p5pRT commented Jun 17, 2008