From marc-philip@die-werners.de

From​: marc-philip@​die-werners.de
To​: perlbug@​perl.org
Message-Id​: <5.26.1_51638_1512643179@​WDFM33972517A>
Reply-To​: marc-philip@​die-werners.de
Subject​: uninitialized variable and integer overrun in pp.c and toke.c

This is a bug report for perl from marc-philip@​die-werners.de,
generated with the help of perlbug 1.40 running under perl 5.26.1.

Hi,
we have some coverity code scans here. They have found this
uninitialized variable in pp.c and the integer overrun in toke.c.
Though it might be possible that these are false positives (no
reasonable control path gets there), it's good to mute the scan here to
better see other (possibly real) problems.

I'm attaching a patchfile.

Best Regards
Marc-Philip

Flags​:
  category=core
  severity=medium

Site configuration information for perl 5.26.1​:

Configured by sap at Thu Dec 7 10​:39​:47 CET 2017.

Summary of my perl5 (revision 5 version 26 subversion 1) configuration​:

  Platform​:
  osname=darwin
  osvers=17.2.0
  archname=darwin-thread-multi-2level
  uname='darwin wdfm33972517a 17.2.0 darwin kernel version 17.2.0​: fri
sep 29 18​:27​:05 pdt 2017; root​:xnu-4570.20.62~3release_x86_64 x86_64 '
  config_args='-der -Dmyhostname=buildhost -Dmydomain=.com -Dcf_by=sap
-Dprivlib=.../../lib -Dsitelib=.../../lib -Darchlib=.../../lib
-Dsitearch=.../../lib -Dnoextensions=GDBM_File NDBM_File ODBM_File
SDBM_File -Ddynamic_ext=MIME/Base64 Socket IO Time/HiRes Cwd Encode
Data/Dumper Compress/Raw/Zlib -Dusethreads -Duseshrplib
-Duserelocatableinc
-Dprefix=/Users/d026948/SAPDevelop/hmexternals/perl/gen/out/perl-5.26.1-sap2-SNAPSHOT-darwinintel64-release-c
-Duse64bitall -Accflags=-B$SDKROOT/usr/include/gcc
-Accflags=-B$SDKROOT/usr/lib/gcc -Accflags=-isystem$SDKROOT/usr/include
-Accflags=-F$SDKROOT/System/Library/Frameworks
-Aldflags=-Wl,-syslibroot,$SDKROOT
-Accdlflags=-B$SDKROOT/usr/include/gcc
-Accdlflags=-B$SDKROOT/usr/lib/gcc
-Accdlflags=-isystem$SDKROOT/usr/include
-Accdlflags=-F$SDKROOT/System/Library/Frameworks
-Alddlflags=-Wl,-syslibroot,$SDKROOT'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.13 -arch
x86_64
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gcc
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/gcc
-isystem/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include
-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks
-fno-strict-aliasing -pipe -fstack-protector-strong
-DPERL_USE_SAFE_PUTENV'
  optimize='-O3'
  cppflags='-arch x86_64 -fno-common -DPERL_DARWIN
-mmacosx-version-min=10.13 -arch x86_64
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gcc
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/gcc
-isystem/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include
-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks
-fno-strict-aliasing -pipe -fstack-protector-strong'
  ccversion=''
  gccversion='4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.38)'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='cc -arch x86_64'
  ldflags =' -mmacosx-version-min=10.13 -arch x86_64
-Wl,-syslibroot,/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
-fstack-protector-strong'
 
libpth=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0/lib
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/lib
/usr/lib
  libs=-lpthread -ldbm -ldl -lm -lutil -lc
  perllibs=-lpthread -ldl -lm -lutil -lc
  libc=
  so=dylib
  useshrplib=true
  libperl=libperl.dylib
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=bundle
  d_dlsymun=undef
  ccdlflags='
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gcc
-B/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/gcc
-isystem/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include
-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks'
  cccdlflags=' '
  lddlflags=' -mmacosx-version-min=10.13 -bundle -undefined
dynamic_lookup
-Wl,-syslibroot,/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
-fstack-protector-strong'

@​INC for perl 5.26.1​:
 
/Users/d026948/SAPDevelop/hmexternals/perl/gen/out/perl-5.26.1-sap2-SNAPSHOT-darwinintel64-release-c/lib
 
/Users/d026948/SAPDevelop/hmexternals/perl/gen/out/perl-5.26.1-sap2-SNAPSHOT-darwinintel64-release-c/lib

Environment for perl 5.26.1​:
 
DYLD_LIBRARY_PATH=/Users/d026948/SAPDevelop/hmexternals/perl/gen/out/perl-5.26.1-sap2-SNAPSHOT-darwinintel64-release-c/lib/CORE
  HOME=/Users/d026948
  LANG=en_GB.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
 
PATH=/Users/d026948/SAPDevelop/hmexternals/perl/gen/out/perl-5.26.1-sap2-SNAPSHOT-darwinintel64-release-c/bin​:/Users/d026948/SAPDevelop/xmake-0.9.3-8/bin​:/opt/local/bin​:/opt/local/sbin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/sbin​:/sbin​:/opt/X11/bin
  PERL_BADLANG (unset)
  SHELL=/bin/csh

From marc-philip@die-werners.de

diff -Naur perl-5.26.1.orig/pp.c perl-5.26.1/pp.c
--- perl-5.26.1.orig/pp.c	2017-08-23 20:22:51.000000000 +0000
+++ perl-5.26.1/pp.c	2017-11-27 15:18:24.000000000 +0000
@@ -3811,6 +3811,7 @@
     if (! slen) {   /* If empty */
 	need = 1; /* still need a trailing NUL */
 	ulen = 0;
+        *tmpbuf = '\0';
     }
     else if (DO_UTF8(source)) {	/* Is the source utf8? */
 	doing_utf8 = TRUE;
diff -Naur perl-5.26.1.orig/toke.c perl-5.26.1/toke.c
--- perl-5.26.1.orig/toke.c	2017-07-18 23:00:00.000000000 +0000
+++ perl-5.26.1/toke.c	2017-11-27 15:33:36.000000000 +0000
@@ -8943,7 +8943,7 @@
 		HEK * const stashname = HvNAME_HEK(stash);
 		SV *  const sym = newSVhek(stashname);
                 sv_catpvs(sym, "::");
-                sv_catpvn_flags(sym, PL_tokenbuf+1, tokenbuf_len - 1, (UTF ? SV_CATUTF8 : SV_CATBYTES ));
+                sv_catpvn_flags(sym, PL_tokenbuf+1, tokenbuf_len > 0 ? tokenbuf_len - 1 : 0, (UTF ? SV_CATUTF8 : SV_CATBYTES ));
                 pl_yylval.opval = newSVOP(OP_CONST, 0, sym);
                 pl_yylval.opval->op_private = OPpCONST_ENTERED;
                 if (pit != '&')
@@ -8971,7 +8971,7 @@
         && PL_lex_state != LEX_NORMAL
         && !PL_lex_brackets)
     {
-        GV *const gv = gv_fetchpvn_flags(PL_tokenbuf + 1, tokenbuf_len - 1,
+        GV *const gv = gv_fetchpvn_flags(PL_tokenbuf + 1, tokenbuf_len > 0 ? tokenbuf_len - 1 : 0,
                                          ( UTF ? SVf_UTF8 : 0 ) | GV_ADDMG,
                                          SVt_PVAV);
         if ((!gv || ((PL_tokenbuf[0] == '@') ? !GvAV(gv) : !GvHV(gv)))
@@ -8988,11 +8988,11 @@
     /* build ops for a bareword */
     pl_yylval.opval = newSVOP(OP_CONST, 0,
 				   newSVpvn_flags(PL_tokenbuf + 1,
-						      tokenbuf_len - 1,
+                                                      tokenbuf_len > 0 ? tokenbuf_len - 1 : 0,
                                                       UTF ? SVf_UTF8 : 0 ));
     pl_yylval.opval->op_private = OPpCONST_ENTERED;
     if (pit != '&')
-	gv_fetchpvn_flags(PL_tokenbuf+1, tokenbuf_len - 1,
+        gv_fetchpvn_flags(PL_tokenbuf+1, tokenbuf_len > 0 ? tokenbuf_len - 1 : 0,
 		     (PL_in_eval ? GV_ADDMULTI : GV_ADD)
                      | ( UTF ? SVf_UTF8 : 0 ),
 		     ((PL_tokenbuf[0] == '$') ? SVt_PV

From @avar

On Thu, Dec 7, 2017 at 1​:00 PM, Marc-Philip <perlbug-followup@​perl.org> wrote​:

we have some coverity code scans here. They have found this
uninitialized variable in pp.c and the integer overrun in toke.c.
Though it might be possible that these are false positives (no
reasonable control path gets there), it's good to mute the scan here to
better see other (possibly real) problems.

Thanks, the first hunk probably looks sensible, although maybe it
should be done when the variable is first allocated (i.e. add a \0 at
the end), but I don't know enough to be sure.

All the rest of the hunks are paranoia that strlen() will return a
negative value. This seems to just be coverty getting confused, the
STRLEN type is eventually defined in terms of size_t, which AFAIK is
always unsigned.

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

From marc-philip.werner@sap.com

On Thu, 07 Dec 2017 06​:01​:35 -0800, avarab@​gmail.com wrote​:

On Thu, Dec 7, 2017 at 1​:00 PM, Marc-Philip <perlbug-
followup@​perl.org> wrote​:

we have some coverity code scans here. They have found this
uninitialized variable in pp.c and the integer overrun in toke.c.
Though it might be possible that these are false positives (no
reasonable control path gets there), it's good to mute the scan here
to
better see other (possibly real) problems.

Thanks, the first hunk probably looks sensible, although maybe it
should be done when the variable is first allocated (i.e. add a \0 at
the end), but I don't know enough to be sure.
wherever you like.

All the rest of the hunks are paranoia that strlen() will return a
negative value. This seems to just be coverty getting confused, the
STRLEN type is eventually defined in terms of size_t, which AFAIK is
always unsigned.
The problem here is when tokenbuf_len==0. 0 is a value that strlen() can return. This does not look like a coverity confusion to me.

Regards
MP

From Eirik-Berg.Hanssen@allverden.no

On Thu, Dec 7, 2017 at 3​:01 PM, Ævar Arnfjörð Bjarmason <avarab@​gmail.com>
wrote​:

On Thu, Dec 7, 2017 at 1​:00 PM, Marc-Philip <perlbug-followup@​perl.org>
wrote​:

we have some coverity code scans here. They have found this
uninitialized variable in pp.c and the integer overrun in toke.c.
Though it might be possible that these are false positives (no
reasonable control path gets there), it's good to mute the scan here to
better see other (possibly real) problems.

Thanks, the first hunk probably looks sensible, although maybe it
should be done when the variable is first allocated (i.e. add a \0 at
the end), but I don't know enough to be sure.

All the rest of the hunks are paranoia that strlen() will return a
negative value. This seems to just be coverty getting confused, the
STRLEN type is eventually defined in terms of size_t, which AFAIK is
always unsigned.

  An unsigned int can still be zero, in which case -1 gives a negative. Or
is that still not worth complaining about?

  (Perhaps testing for !=0 instead of >0 would be clearer? :) )

Eirik

From zefram@fysh.org

Marc-Philip wrote​:

we have some coverity code scans here. They have found this
uninitialized variable in pp.c and the integer overrun in toke.c.

Neither is a real bug. In the empty string case in pp_ucfirst(), tmpbuf
will not be read from. In S_pending_ident(), the function will not be
called with an empty string in PL_tokenbuf (in fact the minimum string
length is 2).

-zefram

Is this closable then

I dont think Zefram is going to answer so I will close it.