Skip to content

Commit ac6a62d

Browse files
committed
find_script: avoid potential integer overflow
Fixes a Coverity issue: >>> function_return: Function Perl_delimcpy_no_escape(tmpbuf, tmpbuf + 4096UL, s, bufend, 58, &len) modifies its argument, assigning 2147483647 to len. 3553 s = delimcpy_no_escape(tmpbuf, tmpbuf + sizeof tmpbuf, s, bufend, 3554 ':', &len); >>> CID 583353: (Perl#1 of 1): Overflowed constant (INTEGER_OVERFLOW) >>> overflow_const: Expression len + 1, where len is known to be equal to 2147483647, overflows the type of len + 1, which is type int. 3558 if (len + 1 + strlen(scriptname) + MAX_EXT_LEN >= sizeof tmpbuf) 3559 continue; /* don't search dir with too-long name */ If there is not enough available space in tmpbuf, delimcpy_no_escape sets len to I32_MAX, but the following code does not check for this. (I believe this case is reachable simply by setting PATH to a huge string.) Avoid the potential overflow by rewriting A + B >= C as A >= C - B (Also, make 'len' unsigned (specifically, size_t) to match the type of sizeof/strlen() and avoid warnings about comparisons between signed and unsigned integers.)
1 parent af22bdc commit ac6a62d

File tree

1 file changed

+18
-5
lines changed

1 file changed

+18
-5
lines changed

util.c

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3410,7 +3410,7 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch,
34103410
char *xfailed = NULL;
34113411
char tmpbuf[MAXPATHLEN];
34123412
char *s;
3413-
I32 len = 0;
3413+
size_t len = 0;
34143414
int retval;
34153415
char *bufend;
34163416
#if defined(DOSISH) && !defined(OS2)
@@ -3550,13 +3550,26 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch,
35503550
if (len < sizeof tmpbuf)
35513551
tmpbuf[len] = '\0';
35523552
# else
3553-
s = delimcpy_no_escape(tmpbuf, tmpbuf + sizeof tmpbuf, s, bufend,
3554-
':', &len);
3553+
{
3554+
I32 n;
3555+
s = delimcpy_no_escape(tmpbuf, tmpbuf + sizeof tmpbuf, s,
3556+
bufend, ':', &n);
3557+
assert(n >= 0);
3558+
len = n;
3559+
}
35553560
# endif
35563561
if (s < bufend)
35573562
s++;
3558-
if (len + 1 + strlen(scriptname) + MAX_EXT_LEN >= sizeof tmpbuf)
3559-
continue; /* don't search dir with too-long name */
3563+
{
3564+
const size_t
3565+
available_len = sizeof tmpbuf - MAX_EXT_LEN - 1,
3566+
scriptname_len = strlen(scriptname);
3567+
if (
3568+
scriptname_len >= available_len ||
3569+
len >= available_len - scriptname_len
3570+
)
3571+
continue; /* don't search dir with too-long name */
3572+
}
35603573
if (len
35613574
# ifdef DOSISH
35623575
&& tmpbuf[len - 1] != '/'

0 commit comments

Comments
 (0)