Skip to content

Commit

Permalink
libpmi: check for overflow in parsing code
Browse files Browse the repository at this point in the history
Problem: github CodeQL calls out a missed overflow check in
users of keyval_parse_uint().

keyval_parse_uint() and keyval_parse_int() assign the result
of strtoul() and strtol() respectively without checking for under/
overflow.

Add checks.
Update unit tests.
  • Loading branch information
garlick committed Sep 5, 2023
1 parent 2342115 commit 66f9d71
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/common/libpmi/keyval.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# include "config.h"
#endif
#include <stdlib.h>
#include <limits.h>
#include <string.h>
#include <ctype.h>
#include <errno.h>
Expand All @@ -36,12 +37,14 @@ int keyval_parse_uint (const char *s, const char *key, unsigned int *val)
{
const char *cp = parse_val (s, key);
char *endptr;
unsigned int i;
unsigned long i;
if (!cp)
return EKV_NOKEY;
errno = 0;
i = strtoul (cp, &endptr, 10);
if (errno != 0 || (*endptr && !isspace (*endptr)))
if (errno != 0
|| (*endptr && !isspace (*endptr))
|| i > UINT_MAX)
return EKV_VAL_PARSE;
*val = i;
return EKV_SUCCESS;
Expand All @@ -51,12 +54,15 @@ int keyval_parse_int (const char *s, const char *key, int *val)
{
const char *cp = parse_val (s, key);
char *endptr;
int i;
long i;
if (!cp)
return EKV_NOKEY;
errno = 0;
i = strtol (cp, &endptr, 10);
if (errno != 0 || (*endptr && !isspace (*endptr)))
if (errno != 0
|| (*endptr && !isspace (*endptr))
|| i > INT_MAX
|| i < INT_MIN)
return EKV_VAL_PARSE;
*val = i;
return EKV_SUCCESS;
Expand Down
21 changes: 21 additions & 0 deletions src/common/libpmi/test/keyval.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,25 @@ static char *spawn[] = {
NULL,
};

void check_overflow (void)
{
int rc;
int i;
unsigned int u;

rc = keyval_parse_int ("x=4294967296", "x", &i);
ok (rc == EKV_VAL_PARSE,
"keyval_parse_int x=2^32 (overflow) fails with EKV_VAL_PARSE");

rc = keyval_parse_int ("x=-4294967296", "x", &i);
ok (rc == EKV_VAL_PARSE,
"keyval_parse_int x=-2^32 (overflow) fails with EKV_VAL_PARSE");

rc = keyval_parse_uint ("x=8589934592", "x", &u);
ok (rc == EKV_VAL_PARSE,
"keyval_parse_uint x=2^33 (overflow) fails with EKV_VAL_PARSE");
}

int main(int argc, char** argv)
{
char val[42];
Expand Down Expand Up @@ -306,6 +325,8 @@ int main(int argc, char** argv)
&& streq (val, "0,0"),
"parsed pmi-1 spawn response");

check_overflow();

done_testing();
}

Expand Down

0 comments on commit 66f9d71

Please sign in to comment.