heap-use-after-free S_gv_fetchmeth_internal (gv.c:782) #15772
Comments
From @geeknikTriggered with Perl v5.25.7-98-gdf13534 while fuzzing with AFL. od -tx1 test775 Can't locate object method "mD" via package "2" (perhaps you forgot to load
|
From @geeknik |
From @tonycozOn Tue, 13 Dec 2016 17:20:50 -0800, brian.carpenter@gmail.com wrote:
Simplifies to: mD{--$|,%x=f..k+--$|,%x=f..k}"abc" which needs to be in a file (perl -e ... doesn't reproduce it). The following: mD{--$|,%x=f..k,%x=f..k}"abc" again in a file, produces: tony@mars:.../git/perl$ ./perl ../130344b.pl The original and its simplified version are crashing while looking $ ./perl -Do ../130344b.pl EXECUTING... (../130344b.pl:1) Looking for method mD in package UNIVERSAL Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @arcOn Wed, 14 Dec 2016 15:45:53 -0800, tonyc wrote:
Further reduction: f { $s=1, %x=j..k+($s=0), %x=() } 9 So I suspect at least part of this is stack-not-refcounted.
I think that's because using -e causes IO::File to have a 0 cached for its DESTROY method before execution starts. I get the same heap-use-after-free under miniperl, fwiw.
With this further reduction, rt130344c.pl: f { $s=1, @x=2, @x=() } 9 I get the same assertion failure as with rt130344b.pl. But that doesn't seem to be the whole of the same bug, in that this fixes it: Inline Patchdiff --git a/gv.c b/gv.c
index 2570cf0657..6f513ca1e2 100644
--- a/gv.c
+++ b/gv.c
@@ -1495,7 +1495,10 @@ S_gv_stashsvpvn_cached(pTHX_ SV *namesv, const char *name, U32 namelen, I32 flag
(flags & SVf_UTF8) ? HVhek_UTF8 : 0, 0, NULL, 0
);
- if (he) return INT2PTR(HV*,SvIVX(HeVAL(he)));
+ if (he) {
+ SV *sv = HeVAL(he);
+ return SvIOK(sv) ? INT2PTR(HV*, SvIVX(sv)) : NULL;
+ }
else if (flags & GV_CACHE_ONLY) return NULL;
if (namesv) {
That change isn't good enough, because running rt130344c.pl with -e still yields "Attempt to free unreferenced scalar". But I think it's probably needed anyway, and I'm wondering whether it should be applied to blead directly. That is, I suspect fixing it wouldn't disclose an unfixed security issue. -- |
From @tonycozOn Sun, 08 Jan 2017 06:14:29 -0800, arc wrote:
The problem I see with this patch is it's more a workaround for the stack-not-refcounted bug rather than a fix for the issue. In the general case that bug can result in re-use of SV pool entries, so while in this particular case it might detect the problem, in the more general that SV could have been re-used and gv_stashsvpvn_cached() will be returning some random SV instead. It really needs an assertion, something like: /* make sure it's a HV, though it might still be the wrong HV */ which I expect would currently only fail due to the stack-not-refcounted bug. Tony |
From @iabynOn Tue, Jan 31, 2017 at 04:22:49PM -0800, Tony Cook via RT wrote:
I agree with Tony in that I think the assertion should be applied rather Then the ticket should be moved to the public queue, added to the 'stack -- |
From @iabynOn Mon, Feb 20, 2017 at 03:25:34PM +0000, Dave Mitchell wrote:
I've now pushed the assert as v5.25.10-42-g78e4f28 and am doing the moving -- |
Migrated from rt.perl.org#130344 (status was 'open')
Searchable as RT130344$
The text was updated successfully, but these errors were encountered: