Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

ExtUtils::CBuilder fails to compile modules with warnings when -Wall is part of Perl's compile time flags #22212

Closed rlauer6 closed 4 months ago

rlauer6 commented 6 months ago

Update

The issue was actually the -Wformat=security flag. The code was not using a literal for the printf statement. Modifying the source to pass a literal "%s" as the first arg to croak allowed the module to build correctly. However I'm still not sure how to remove flags from the compiler options (even if that turns out to be a bad idea).

Module: ExtUtils::CBuilder::Base

Description Using the latest Amazon Linux container (amazonlinux:latest) ...

While trying to compile Archive::Extract::Libarchive, compiling fails because the perl supplied in their repo is compiled with the -Wall flag which is included when Module::Buildattempts to compile the module (and there are warnings when compiling).

Removing -Wall right before Module::Build runs the compiler allows the module to proceed..

Not sure if this is a bug in Module::Build, ExtUtils::CBuilder::Base or just a deficiency of the tool chain's ability to remove flags that might cause downstream issues? In any event, there are many modules that compile with warnings so this may be a bigger issue when trying to install modules on the latest version of Amazon Linux.

Steps to Reproduce

Perl configuration

bash-5.2# perl -V
Summary of my perl5 (revision 5 version 32 subversion 1) configuration:

  Platform:
    osname=linux
    osvers=4.14.327-246.539.amzn2.x86_64
    archname=x86_64-linux-thread-multi
    uname='linux ip-10-0-56-0.us-west-2.compute.internal 4.14.327-246.539.amzn2.x86_64 #1 smp sun oct 22 17:17:17 utc 2023 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=none -Dccflags=-O2 -ftree-vectorize -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe **-Wall** -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Dldflags=-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1  -Dccdlflags=-Wl,--enable-new-dtags -Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1  -Dlddlflags=-shared -Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1  -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.32.1 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Amazon Linux -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5/5.32 -Dsitearch=/usr/local/lib64/perl5/5.32 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize -Duse64bitint'
    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='gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -O2 -ftree-vectorize -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fwrapv -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='  -g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -O2 -ftree-vectorize -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fwrapv -fno-strict-aliasing -I/usr/local/include'                      
    ccversion=''
    gccversion='11.4.1 20230605 (Red Hat 11.4.1-2)'
    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='gcc'
    ldflags ='-Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--build-id=sha1  -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib64 /lib64 /usr/lib64 /usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib
    libs=-lpthread -lresolv -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lresolv -ldl -lm -lcrypt -lutil -lc
    libc=/lib/../lib64/libc.so.6
    so=so
    useshrplib=true
    libperl=libperl.so
    gnulibc_version='2.34'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,--enable-new-dtags -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--build-id=sha1 '
    cccdlflags='-fPIC'
    lddlflags='-lpthread -shared -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--build-id=sha1  -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl):
  Compile-time options:
    HAS_TIMES
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_IMPLICIT_CONTEXT
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_REENTRANT_API
    USE_SITECUSTOMIZE
    USE_THREAD_SAFE_LOCALE
  Locally applied patches:
    Fedora Patch1: Removes date check, Fedora/RHEL specific
    Fedora Patch2: support for libdir64
    Fedora Patch3: use libresolv instead of libbind
    Fedora Patch4: USE_MM_LD_RUN_PATH
    Fedora Patch5: Provide MM::maybe_command independently (bug #1129443)
    Fedora Patch6: Dont run one io test due to random builder failures
    Fedora Patch8: Define SONAME for libperl.so
    Fedora Patch9: Install libperl.so to -Dshrpdir value
    Fedora Patch10: Make *DBM_File desctructors thread-safe (RT#61912)
    Fedora Patch11: Replace EU::MakeMaker dependency with EU::MM::Utils in IPC::Cmd (bug #1129443)
    Fedora Patch12: Link XS modules to pthread library to fix linking with -z defs
    Fedora Patch13: Pass the correct CFLAGS to dtrace
    Fedora Patch14: Do not use C compiler reserved identifiers
    Fedora Patch15: Fix SvUV_nomg() macro definition
    Fedora Patch16: Fix SvTRUE() documentation
    Fedora Patch17: Fix ext/XS-APItest/t/utf8_warn_base.pl tests
    Fedora Patch18: Fix IO::Handle::error() to report write errors (GH#6799)
    Fedora Patch19: Fix IO::Handle::error() to report write errors (GH#6799)
    Fedora Patch21: Fix setting a non-blocking mode in IO::Socket::UNIX (GH#17787)
    Fedora Patch22: Fix running actions after stepping in a debugger (GH#17901)
    Fedora Patch23: Fix running actions after stepping in a debugger (GH#17901)
    Fedora Patch24: Fix running actions after stepping in a debugger (GH#17901)
    Fedora Patch25: Fix a buffer size for asctime_r() and ctime_r() functions
    Fedora Patch26: Prevent from an integer overflow in RenewDouble() macro
    Fedora Patch28: Fix a number of arguments passed to a BOOT XS subroutine (GH#17755)
    Fedora Patch29: Fix an IO::Handle spurious error reported for regular file handles (GH#18019)
    Fedora Patch30: Fix inheritance resolution of lexial objects in a debugger (GH#17661)
    Fedora Patch35: Fix sorting with a block that calls return (GH#18081)
    Fedora Patch38: Fix sv_collxfrm macro to respect locale
    Fedora Patch39: Fix an iterator signedness in handling an mro exception (GH#18155)
    Fedora Patch40: Fix a code flow in Perl_sv_inc_nomg()
    Fedora Patch41: Fix an undefined behavior in Perl_custom_op_get_field()
    Fedora Patch42: Fix Config variable names in in t/op tests
    Fedora Patch43: Fix fetching a magic on the stacked file test operators
    Fedora Patch44: Fix a crash in optimizing split() (GH#18232)
    Fedora Patch45: Fix a crash in optimizing split() (GH#18232)
    Fedora Patch46: Fix a crash in optimizing split() (GH#18232)
    Fedora Patch47: Fix a crash in optimizing split() (GH#18232)
    Fedora Patch48: Make accessing environment by DynaLoader thread-safe
    Fedora Patch49: Use duplocale() if available
    Fedora Patch50: Fix fc() in Turkish locale
    Fedora Patch51: Fix croaking on "my $_" when "use utf8" is in effect (GH#18449)
    Fedora Patch52: Fix PERL_UNUSED_ARG() definition in XSUB.h
    Fedora Patch53: Add missing entries to perldiag (GH#18276)
    Fedora Patch54: Protect locale tests from LANGUAGE environment variable
    Fedora Patch55: Prevent the number of buckets in a hash from getting too large
    Fedora Patch56: Fix a memory leak when compiling a regular expression (GH#18604)
    Fedora Patch57: Fix dumping a hash entry of PL_strtab type
    Fedora Patch57: Fix an arithmetic left shift of a minimal integer value (GH#18639)
    Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux
    Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux
  Built under linux
  Compiled at Nov 28 2023 00:00:00
  %ENV:
    PERL_HTTP_TINY_SSL_INSECURE_BY_DEFAULT="0"
  @INC:
    /usr/local/lib64/perl5/5.32
    /usr/local/share/perl5/5.32
    /usr/lib64/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib64/perl5
    /usr/share/perl5
bash-5.2#
Leont commented 6 months ago

-Wall shouldn't cause any compilation to fail, it should cause warnings to be be printed. Can you paste the full output of the build?

rlauer6 commented 6 months ago

Yes, as noted -Wall was not the issue and only printed all of the errors, the offending flag was `-Wformat=security'. Diving into that module's code a bit I found that modifying the code solved the problem. Essentially the compiler was correctly pointing out the security issue regarding passing a non-string literal to printf. I'm still curious as to how one could "turn off flags", although in this case the flag was there to harden the module a bit.

Leont commented 6 months ago

Yes, as noted -Wall was not the issue and only printed all of the errors, the offending flag was `-Wformat=security'. Diving

No, the offending flag is -Werror=format-security, which actually does make sense. The whole point of -Werror is to make warnings fatal. It's explicitly asking for this.

Diving into that module's code a bit I found that modifying the code solved the problem. Essentially the compiler was correctly pointing out the security issue regarding passing a non-string literal to printf.

Yeah prepending a "%s", in a few places should solve the entire issue. I'm not sure if it's really a security issue (that depends on how much the attacker can affect the error message itself), but it's definitely something that should be changed. This should probably be fixed upstream but I'm not sure the author is still around so that might require someone taking over the module.

I'm still curious as to how one could "turn off flags", although in this case the flag was there to harden the module a bit.

You can't easily do that, except by overriding ccflags completely (e.g. perl Build.PL --config ccflags="...")

rlauer6 commented 6 months ago

No, the offending flag is -Werror=format-security, which actually does make sense. The whole point of -Werror is to make warnings fatal. It's explicitly asking for this.

Yeah -Werror=format-security forces the compiler to consider that warning an error, I was typing that from memory...been looking cross-eyed at building a bunch of stuff in Amazon Linux (AL2023) - what a nightmare!

You can't easily do that, except by overriding ccflags completely (e.g. perl Build.PL --config ccflags="...")

Thanks for the replies - at least I know I'm not missing anything wrt the flags. But it seems almost impossible to override them completely unless I am missing something. No matter how hard I tried to override the flags it seemed that somewhere new flags were added - perhaps I didn't try supplying them on the command line to Build.PL. The journey started of course with cpanm so I'll have to see that works.

Thanks again

jkeenan commented 4 months ago

@Leont, it appears that you have addressed the OP's concerns and there has been no further discussion in this ticket since May 13. I'll close it in a week unless you think there's some reason to keep it open.