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
Out of bounds read when calling perl from C #15806
Comments
From hanno@hboeck.deCreated by hanno@hboeck.deOut of bounds read when calling perl from C When calling perl from C with parameters an out of bounds array access Here's some example code triggering that bug: When you run that with valgrind (alternatively also with perl built The problem is this code (perl.c line 2165/2166, perl-5.24.1-RC4): This assumes argv points to the beginning of the parameter array. A few lines earlier (line 1888) we can see this: This loop will let the argv pointer point to the end of the parameter I think the proper fix is to replace argv[0] with PL_origargv[0] in Perl Info
|
From @jkeenanOn Fri, 13 Jan 2017 15:54:24 GMT, hanno@hboeck.de wrote:
Making the change you suggested -- PL_origargv[0] -- and no other, I attempted to build Perl 5 blead. 'make' died on both FreeBSD and Linux at this point: ##### See smoke-me/jkeenan/130550-out-of-bounds branch. Thank you very much. -- |
The RT System itself - Status changed from 'new' to 'open' |
From @hvdsOn Fri, 13 Jan 2017 07:54:24 -0800, hanno@hboeck.de wrote:
Not quite - if you run "perl -Ilib foo", it wants to pick up the scriptname 'foo'. However it shouldn't attempt to pick that up if there are no more arguments. I suspect all platforms tend to terminate the argv array with a spare NULL, else we'd surely have seen this before: that behaviour has been in the code for at least 20 years. I think the fix should look something like the below, I'll try to wrap a test around it. The later uses of arg{c,v} also look dubious, I'll look at those some more as well. Hugo [perl #130550] don't read non-existent scriptname Inline Patchdiff --git a/perl.c b/perl.c
index 09eb2f4..0ee6730 100644
--- a/perl.c
+++ b/perl.c
@@ -2206,18 +2206,20 @@ S_parse_body(pTHX_ char **env, XSINIT_t xsinit)
}
#endif
- if (!scriptname)
- scriptname = argv[0];
if (PL_e_script) {
argc++,argv--;
scriptname = BIT_BUCKET; /* don't look for script or read stdin */
}
else if (scriptname == NULL) {
+ if (argc > 0)
+ scriptname = argv[0];
+ else {
#ifdef MSDOS
- if ( PerlLIO_isatty(PerlIO_fileno(PerlIO_stdin())) )
- moreswitches("h");
+ if ( PerlLIO_isatty(PerlIO_fileno(PerlIO_stdin())) )
+ moreswitches("h");
#endif
- scriptname = "-";
+ scriptname = "-";
+ }
}
assert (!TAINT_get); |
From @iabyn
From perlembed.pod: Mind that argv[argc] must be NULL, same as those passed to a main You need an extra NULL in your args list. -- |
From @hvdsOn Sat, 14 Jan 2017 03:34:43 -0800, davem wrote:
Is there any reason we _should_ require that, other than that it would require us to tighten up our argv/argc handling? It feels like a somewhat unperlish stipulation. If for example we're sometimes handing it off to libc calls that already require it then fair enough, but I'm not aware that we're doing anything like that. Hugo |
From @iabynOn Sat, Jan 14, 2017 at 12:11:02PM -0800, Hugo van der Sanden via RT wrote:
We assign argv to PL_origargv, which is then available for the rest of -- |
From hanno@hboeck.deOn Sat, 14 Jan 2017 03:34:43 -0800
Ah, sorry... |
From hanno@hboeck.deOn Fri, 13 Jan 2017 18:21:01 -0800
Yeah, you're right, it seems it's not as simple as I thought on first -- mail/jabber: hanno@hboeck.de |
From hanno@hboeck.deOn Sun, 15 Jan 2017 02:59:55 -0800
If there's an expectation by modules then I'd actually argue that it's I think there's nothing to do here and it was my mistake having misread (By the way: I found this bug because irssi had a non-null-terminated -- mail/jabber: hanno@hboeck.de |
From @iabynOn Mon, Jan 16, 2017 at 11:02:01AM +0100, Hanno Böck wrote:
In fact, perhaps parse_body() should have an explicit assert(!argv[argc]) -- |
From zefram@fysh.orgThe need for null argv[argc] was more fully documented in commit -zefram |
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#130550 (status was 'resolved')
Searchable as RT130550$
The text was updated successfully, but these errors were encountered: