Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic dereferencing in the version object code allows SEGVs #10705

Closed
p5pRT opened this issue Oct 8, 2010 · 4 comments
Closed

Automatic dereferencing in the version object code allows SEGVs #10705

p5pRT opened this issue Oct 8, 2010 · 4 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Oct 8, 2010

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

Searchable as RT78286$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 8, 2010

From @nwc10

Created by @nwc10

Perl_vverify contains

  if ( SvROK(vs) )
  vs = SvRV(vs);

before it verifies whether what it is passed meets the structural
requirements.

However, vverify is called after a similar automatic dereferencing in
Perl_vstringify (and Perl_vnumify)

The result is that vverify will *pass* something that should *fail*,
so instead of correctly croaking, its callers will continue onwards and
fail less gracefully​:

$ ./perl -lwe '$a = \\version->new(1); bless $a, "version"; print $a'
Segmentation fault

Whilst the "obvious" fix is to re-order the automatic dereference and the
call to vverify() in the callers, I believe a better fix would be to audit
the whole approach to automatic dereferencing, to validate that objects passed
in can only be dereferenced exactly zero or exactly once, whatever code path
is taken. Moreover, it likely might be better to change the implementation
to have the public API entry points dereference and validate *once*, and
then call internal functions that do not dereference, and do not repeatedly
revalidate.

(The current code has paths that revalidate. This is wasteful.)

Nicholas Clark

Perl Info

Flags:
    category=library
    severity=low
    module=version

Site configuration information for perl 5.13.5:

Configured by nick at Fri Oct  8 13:45:33 BST 2010.

Summary of my perl5 (revision 5 version 13 subversion 5) configuration:
  Derived from: 4c43faf178da84900d2d5ff3de31d38bf7fab5d3
  Platform:
    osname=linux, osvers=2.6.35.4, archname=x86_64-linux-ld
    uname='linux eris 2.6.35.4 #4 smp tue sep 21 09:54:22 bst 2010 x86_64 gnulinux '
    config_args='-Dusedevel=y -Dcc=ccache gcc -Dld=gcc -Ubincompat5005 -Uinstallusrbinperl -Dcf_email=nick@ccl4.org -Dperladmin=nick@ccl4.org -Dinc_version_list=  -Dinc_version_list_init=0 -Doptimize=-Os -Uusethreads -Duselongdouble -Uuse64bitall -Uusemymalloc -Duseperlio -Dprefix=~/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf -Uusevendorprefix -Uvendorprefix=~/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf -Dinstallman1dir=none -Dinstallman3dir=none -Uuserelocatableinc -Umad -Accccflags-DPURIFY -de'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=undef, uselongdouble=define
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='ccache gcc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-Os',
    cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.3.2', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t', lseeksize=8
    alignbytes=16, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
    libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.7.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.7'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -Os -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.13.5:
    lib
    /home/nick/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf/lib/perl5/site_perl/5.13.5/x86_64-linux-ld
    /home/nick/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf/lib/perl5/site_perl/5.13.5
    /home/nick/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf/lib/perl5/5.13.5/x86_64-linux-ld
    /home/nick/Sandpit/snap5.9.x-v5.13.5-322-g4c43faf/lib/perl5/5.13.5
    .


Environment for perl 5.13.5:
    HOME=/home/nick
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/nick/bin:/usr/local/bin:/usr/bin:/bin:/usr/games:/usr/local/sbin:/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

Loading

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 8, 2010

From @xdg

Fixed with the commit 5de8bff

  Change vverify() to return HV or NULL (RT#78286)
 
  Multiple code paths were dereferencing version objects without
  checking the underlying type, which could result in segmentation
  faults per RT#78286
 
  This patch consolidates all dereferencing into vverify() and
  has vverify return the underlying HV or NULL instead of
  a boolean value.

Loading

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 8, 2010

From [Unknown Contact. See original ticket]

Fixed with the commit 5de8bff

  Change vverify() to return HV or NULL (RT#78286)
 
  Multiple code paths were dereferencing version objects without
  checking the underlying type, which could result in segmentation
  faults per RT#78286
 
  This patch consolidates all dereferencing into vverify() and
  has vverify return the underlying HV or NULL instead of
  a boolean value.

Loading

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 8, 2010

@xdg - Status changed from 'new' to 'resolved'

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant