From 5737d31aac51360cc1eb412ef059e36147c9d6d6 Mon Sep 17 00:00:00 2001 From: David Mitchell Date: Fri, 29 Jun 2018 13:37:03 +0100 Subject: Perl_my_setenv(); handle integer wrap RT #133204 Wean this function off int/I32 and onto UV/Size_t. Also, replace all malloc-ish calls with a wrapper that does overflow checks, In particular, it was doing (nlen + vlen + 2) which could wrap when the combined length of the environment variable name and value exceeded around 0x7fffffff. The wrapper check function is probably overkill, but belt and braces... NB this function has several variant parts, #ifdef'ed by platform type; I have blindly changed the parts that aren't compiled under linux. (cherry picked from commit 34716e2a6ee2af96078d62b065b7785c001194be) --- util.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 23 deletions(-) diff --git a/util.c b/util.c index 2e053a7115..ba5fb2ded8 100644 --- a/util.c +++ b/util.c @@ -2064,8 +2064,40 @@ Perl_new_warnings_bitfield(pTHX_ STRLEN *buffer, const char *const bits, *(s+(nlen+1+vlen)) = '\0' #ifdef USE_ENVIRON_ARRAY - /* VMS' my_setenv() is in vms.c */ + +/* small wrapper for use by Perl_my_setenv that mallocs, or reallocs if + * 'current' is non-null, with up to three sizes that are added together. + * It handles integer overflow. + */ +static char * +S_env_alloc(void *current, Size_t l1, Size_t l2, Size_t l3, Size_t size) +{ + void *p; + Size_t sl, l = l1 + l2; + + if (l < l2) + goto panic; + l += l3; + if (l < l3) + goto panic; + sl = l * size; + if (sl < l) + goto panic; + + p = current + ? safesysrealloc(current, sl) + : safesysmalloc(sl); + if (p) + return (char*)p; + + panic: + croak_memory_wrap(); +} + + +/* VMS' my_setenv() is in vms.c */ #if !defined(WIN32) && !defined(NETWARE) + void Perl_my_setenv(pTHX_ const char *nam, const char *val) { @@ -2081,28 +2113,27 @@ Perl_my_setenv(pTHX_ const char *nam, const char *val) #ifndef PERL_USE_SAFE_PUTENV if (!PL_use_safe_putenv) { /* most putenv()s leak, so we manipulate environ directly */ - I32 i; - const I32 len = strlen(nam); - int nlen, vlen; + UV i; + Size_t vlen, nlen = strlen(nam); /* where does it go? */ for (i = 0; environ[i]; i++) { - if (strnEQ(environ[i],nam,len) && environ[i][len] == '=') + if (strnEQ(environ[i], nam, nlen) && environ[i][nlen] == '=') break; } if (environ == PL_origenviron) { /* need we copy environment? */ - I32 j; - I32 max; + UV j, max; char **tmpenv; max = i; while (environ[max]) max++; - tmpenv = (char**)safesysmalloc((max+2) * sizeof(char*)); + /* XXX shouldn't that be max+1 rather than max+2 ??? - DAPM */ + tmpenv = (char**)S_env_alloc(NULL, max, 2, 0, sizeof(char*)); for (j=0; j