develooper Front page | perl.perl5.porters | Postings from April 2014

[perl #121743] [PATCH] Coverity: check for negative return values from/to library calls

Thread Previous
From:
Tony Cook via RT
Date:
April 28, 2014 03:25
Subject:
[perl #121743] [PATCH] Coverity: check for negative return values from/to library calls
Message ID:
rt-4.0.18-23117-1398655508-519.121743-15-0@perl.org
On Sat Apr 26 12:58:05 2014, jhi wrote:
> Dozens of too trusting fileno calls (and then using the returned fds for 
> fstat etc.), plus two similar cases for getgroups().
> 
> Attached.

Fails to build with -Duseithreads due to missing aTHX_ in:

        if (fd < 0) {
            Perl_croak("Illegal suidscript");
        } else {
            if (PerlLIO_fstat(fd, &PL_statbuf) < 0) {	/* may be either wrapped or real suid */
                Perl_croak("Illegal suidscript");
            }
        }

Some other issues, the original code here for example (doio.c):

@@ -1775,8 +1797,9 @@ Perl_apply(pTHX_ I32 type, SV **mark, SV **sp)
                if ((gv = MAYBE_DEREF_GV(*mark))) {
                    if (GvIO(gv) && IoIFP(GvIOp(gv))) {
 #ifdef HAS_FCHOWN
+                        int fd = PerlIO_fileno(IoIFP(GvIOn(gv)));
                        APPLY_TAINT_PROPER();
-                       if (fchown(PerlIO_fileno(IoIFP(GvIOn(gv))), val, val2))
+                       if (fd >= 0 && fchown(fd, val, val2))
                            tot--;
 #else
                        Perl_die(aTHX_ PL_no_func, "fchown");

will sensibly set errno to EBADF when fchown() fails, but the modified code doesn't.

Similarly for the others in Perl_apply(), pp_sysread(), pp_syswrite(), pp_truncate(), pp_getpeername(), pp_stat(), pp_fttty(), pp_fttext() and possibly for PerlIO::mmap.

Tony

---
via perlbug:  queue: perl5 status: new
https://rt.perl.org/Ticket/Display.html?id=121743

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About