mirror/emacs
1
0
Fork 0
mirror of https://git.savannah.gnu.org/git/emacs.git synced 2024-12-30 11:09:23 +00:00
Code Issues Releases Activity

update-game-score fixes for -m and integer overflow

Browse Source 
* update-game-score.c: Include inttypes.h, stdbool.h.
(min): New macro, if not already defined.
(MAX_SCORES, main): Limit the maximum number of scores only from
limits imposed by the underyling platform, instead of the
arbitrary value 200.
(struct score_entry, main, read_score, write_score):
Scores are now intmax_t, not long.
(get_user_id): Reject user names containing spaces or newlines,
as they would mess up the score file.
Allow uids that don't fit in 'long'.
Increase the size of the buffer, to avoid overrun in weird cases.
(get_prefix, main): Use bool for boolean.
(main): Rewrite expr to avoid possibility of signed integer
overflow.  Don't allow newlines in data, as this would mess up
the score file.  Check for memory allocation failure when adding
the new score, or when unlockint the file.  Implement -m.
(read_score): Check for integer overflow when reading a score.
(read_score) [!HAVE_GETDELIM]: Check for integer overflow when
data gets very long.  Check only for space to delimit names,
since that's what's done in the HAVE_GETDELIM case.
(read_scores): New parameter ALLOC.  Change counts to ptrdiff_t.
All uses changed.  Use push_score to add individual scores;
that's simpler than repeating its contents.
(score_compare_reverse): Simplify.
(push_score): New parameter SIZE.  Change counts to ptrdiff_t.
All uses changed.  Check for integer overflow of size calculation.
(sort_scores, write_scores): Change counts to ptrdiff_t.
(unlock_file): Preserve errno on success, so that storage
exhaustion is diagnosed correctly.

Fixes: debbugs:16428
This commit is contained in:
Paul Eggert 2014-01-19 00:50:53 -08:00
parent 9a685cf6bd
commit d70efef4a8
2 changed files with 169 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
lib-src/ChangeLog
View File

@ -1,3 +1,36 @@
2014-01-19 Paul Eggert <eggert@cs.ucla.edu>
update-game-score fixes for -m and integer overflow (Bug#16428)
* update-game-score.c: Include inttypes.h, stdbool.h.
(min): New macro, if not already defined.
(MAX_SCORES, main): Limit the maximum number of scores only from
limits imposed by the underyling platform, instead of the
arbitrary value 200.
(struct score_entry, main, read_score, write_score):
Scores are now intmax_t, not long.
(get_user_id): Reject user names containing spaces or newlines,
as they would mess up the score file.
Allow uids that don't fit in 'long'.
Increase the size of the buffer, to avoid overrun in weird cases.
(get_prefix, main): Use bool for boolean.
(main): Rewrite expr to avoid possibility of signed integer
overflow. Don't allow newlines in data, as this would mess up
the score file. Check for memory allocation failure when adding
the new score, or when unlockint the file. Implement -m.
(read_score): Check for integer overflow when reading a score.
(read_score) [!HAVE_GETDELIM]: Check for integer overflow when
data gets very long. Check only for space to delimit names,
since that's what's done in the HAVE_GETDELIM case.
(read_scores): New parameter ALLOC. Change counts to ptrdiff_t.
All uses changed. Use push_score to add individual scores;
that's simpler than repeating its contents.
(score_compare_reverse): Simplify.
(push_score): New parameter SIZE. Change counts to ptrdiff_t.
All uses changed. Check for integer overflow of size calculation.
(sort_scores, write_scores): Change counts to ptrdiff_t.
(unlock_file): Preserve errno on success, so that storage
exhaustion is diagnosed correctly.
2014-01-05 Paul Eggert <eggert@cs.ucla.edu>
Spelling fixes.

221
lib-src/update-game-score.c
View File

@ -35,7 +35,9 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#include <unistd.h>
#include <errno.h>
#include <inttypes.h>
#include <limits.h>
#include <stdbool.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
@ -50,8 +52,11 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#include "ntlib.h"
#endif
#ifndef min
# define min(a,b) ((a) < (b) ? (a) : (b))
#endif
#define MAX_ATTEMPTS 5
#define MAX_SCORES 200
#define MAX_DATA_LEN 1024
#ifndef HAVE_DIFFTIME
@ -76,18 +81,21 @@ static int unlock_file (const char *filename, void *state);
struct score_entry
{
long score;
intmax_t score;
char *username;
char *data;
};
#define MAX_SCORES min (PTRDIFF_MAX, SIZE_MAX / sizeof (struct score_entry))
static int read_scores (const char *filename, struct score_entry **scores,
int *count);
static int push_score (struct score_entry **scores, int *count,
int newscore, char *username, char *newdata);
static void sort_scores (struct score_entry *scores, int count, int reverse);
ptrdiff_t *count, ptrdiff_t *alloc);
static int push_score (struct score_entry **scores, ptrdiff_t *count,
ptrdiff_t *size, struct score_entry const *newscore);
static void sort_scores (struct score_entry *scores, ptrdiff_t count,
bool reverse);
static int write_scores (const char *filename,
const struct score_entry *scores, int count);
const struct score_entry *scores, ptrdiff_t count);
static _Noreturn void
lose (const char *msg)
@ -107,19 +115,19 @@ static char *
get_user_id (void)
{
struct passwd *buf = getpwuid (getuid ());
if (!buf)
if (!buf || strchr (buf->pw_name, ' ') || strchr (buf->pw_name, '\n'))
{
long uid = getuid ();
char *name = malloc (sizeof uid * CHAR_BIT / 3 + 1);
intmax_t uid = getuid ();
char *name = malloc (sizeof uid * CHAR_BIT / 3 + 4);
if (name)
sprintf (name, "%ld", uid);
sprintf (name, "%"PRIdMAX, uid);
return name;
}
return buf->pw_name;
}
static const char *
get_prefix (int running_suid, const char *user_prefix)
get_prefix (bool running_suid, const char *user_prefix)
{
if (!running_suid && user_prefix == NULL)
lose ("Not using a shared game directory, and no prefix given.");
@ -137,14 +145,18 @@ get_prefix (int running_suid, const char *user_prefix)
int
main (int argc, char **argv)
{
int c, running_suid;
int c;
bool running_suid;
void *lockstate;
char *user_id, *scorefile;
char *scorefile;
char *nl;
const char *prefix, *user_prefix = NULL;
struct stat buf;
struct score_entry *scores;
int newscore, scorecount, reverse = 0, max = MAX_SCORES;
char *newdata;
struct score_entry newscore;
bool reverse = false;
ptrdiff_t scorecount, scorealloc;
ptrdiff_t max_scores = MAX_SCORES;
srand (time (0));
@ -161,15 +173,18 @@ main (int argc, char **argv)
reverse = 1;
break;
case 'm':
max = atoi (optarg);
if (max > MAX_SCORES)
max = MAX_SCORES;
{
intmax_t m = strtoimax (optarg, 0, 10);
if (m < 0)
usage (EXIT_FAILURE);
max_scores = min (m, MAX_SCORES);
}
break;
default:
usage (EXIT_FAILURE);
}
if (optind+3 != argc)
if (argc - optind != 3)
usage (EXIT_FAILURE);
running_suid = (getuid () != geteuid ());
@ -183,13 +198,18 @@ main (int argc, char **argv)
strcpy (scorefile, prefix);
strcat (scorefile, "/");
strcat (scorefile, argv[optind]);
newscore = atoi (argv[optind+1]);
newdata = argv[optind+2];
if (strlen (newdata) > MAX_DATA_LEN)
newdata[MAX_DATA_LEN] = '\0';
user_id = get_user_id ();
if (user_id == NULL)
newscore.score = strtoimax (argv[optind + 1], 0, 10);
newscore.data = argv[optind + 2];
if (strlen (newscore.data) > MAX_DATA_LEN)
newscore.data[MAX_DATA_LEN] = '\0';
nl = strchr (newscore.data, '\n');
if (nl)
*nl = '\0';
newscore.username = get_user_id ();
if (! newscore.username)
lose_syserr ("Couldn't determine user id");
if (stat (scorefile, &buf) < 0)
@ -198,29 +218,34 @@ main (int argc, char **argv)
if (lock_file (scorefile, &lockstate) < 0)
lose_syserr ("Failed to lock scores file");
if (read_scores (scorefile, &scores, &scorecount) < 0)
if (read_scores (scorefile, &scores, &scorecount, &scorealloc) < 0)
{
unlock_file (scorefile, lockstate);
lose_syserr ("Failed to read scores file");
}
push_score (&scores, &scorecount, newscore, user_id, newdata);
if (push_score (&scores, &scorecount, &scorealloc, &newscore) < 0)
{
unlock_file (scorefile, lockstate);
lose_syserr ("Failed to add score");
}
sort_scores (scores, scorecount, reverse);
/* Limit the number of scores. If we're using reverse sorting, then
also increment the beginning of the array, to skip over the
*smallest* scores. Otherwise, just decrementing the number of
scores suffices, since the smallest is at the end. */
if (scorecount > MAX_SCORES)
if (scorecount > max_scores)
{
if (reverse)
scores += (scorecount - MAX_SCORES);
scorecount = MAX_SCORES;
scores += scorecount - max_scores;
scorecount = max_scores;
}
if (write_scores (scorefile, scores, scorecount) < 0)
{
unlock_file (scorefile, lockstate);
lose_syserr ("Failed to write scores file");
}
unlock_file (scorefile, lockstate);
if (unlock_file (scorefile, lockstate) < 0)
lose_syserr ("Failed to unlock scores file");
exit (EXIT_SUCCESS);
}
@ -234,8 +259,12 @@ read_score (FILE *f, struct score_entry *score)
return 1;
for (score->score = 0; (c = getc (f)) != EOF && isdigit (c); )
{
if (INTMAX_MAX / 10 < score->score)
return -1;
score->score *= 10;
score->score += (c-48);
if (INTMAX_MAX - (c - '0') < score->score)
return -1;
score->score += c - '0';
}
while ((c = getc (f)) != EOF
&& isspace (c))
@ -254,18 +283,30 @@ read_score (FILE *f, struct score_entry *score)
}
#else
{
int unameread = 0;
int unamelen = 30;
ptrdiff_t unameread = 0;
ptrdiff_t unamelen = 30;
char *username = malloc (unamelen);
if (!username)
return -1;
while ((c = getc (f)) != EOF
&& !isspace (c))
while ((c = getc (f)) != EOF && c != ' ')
{
if (unameread >= unamelen-1)
if (!(username = realloc (username, unamelen *= 2)))
return -1;
if (unameread >= unamelen - 1)
{
ptrdiff_t unamelen_max = min (PTRDIFF_MAX, SIZE_MAX);
if (unamelen <= unamelen_max / 2)
unamelen *= 2;
else if (unamelen < unamelen_max)
unamelen = unamelen_max;
else
{
errno = ENOMEM;
return -1;
}
username = realloc (username, unamelen);
if (!username)
return -1;
}
username[unameread] = c;
unameread++;
}
@ -286,8 +327,8 @@ read_score (FILE *f, struct score_entry *score)
}
#else
{
int cur = 0;
int len = 16;
ptrdiff_t cur = 0;
ptrdiff_t len = 16;
char *buf = malloc (len);
if (!buf)
return -1;
@ -296,6 +337,11 @@ read_score (FILE *f, struct score_entry *score)
{
if (cur >= len-1)
{
if (min (PTRDIFF_MAX, SIZE_MAX) / 2 < len)
{
errno = ENOMEM;
return -1;
}
if (!(buf = realloc (buf, len *= 2)))
return -1;
}
@ -310,35 +356,25 @@ read_score (FILE *f, struct score_entry *score)
}
static int
read_scores (const char *filename, struct score_entry **scores, int *count)
read_scores (const char *filename, struct score_entry **scores,
ptrdiff_t *count, ptrdiff_t *alloc)
{
int readval = -1, scorecount, cursize;
struct score_entry *ret;
int readval = -1;
ptrdiff_t scorecount = 0;
ptrdiff_t cursize = 0;
struct score_entry *ret = 0;
struct score_entry entry;
FILE *f = fopen (filename, "r");
int retval = -1;
if (!f)
return -1;
scorecount = 0;
cursize = 16;
ret = (struct score_entry *) malloc (sizeof (struct score_entry) * cursize);
if (ret)
{
while ((readval = read_score (f, &ret[scorecount])) == 0)
{
scorecount++;
if (scorecount >= cursize)
{
cursize *= 2;
ret = (struct score_entry *)
realloc (ret, (sizeof (struct score_entry) * cursize));
if (!ret)
break;
}
}
}
while ((readval = read_score (f, &entry)) == 0)
if (push_score (&ret, &scorecount, &cursize, &entry) < 0)
return -1;
if (readval > 0)
{
*count = scorecount;
*alloc = cursize;
*scores = ret;
retval = 0;
}
@ -357,40 +393,53 @@ score_compare (const void *a, const void *b)
static int
score_compare_reverse (const void *a, const void *b)
{
const struct score_entry *sa = (const struct score_entry *) a;
const struct score_entry *sb = (const struct score_entry *) b;
return (sa->score > sb->score) - (sa->score < sb->score);
return score_compare (b, a);
}
int
push_score (struct score_entry **scores, int *count, int newscore, char *username, char *newdata)
push_score (struct score_entry **scores, ptrdiff_t *count, ptrdiff_t *size,
struct score_entry const *newscore)
{
struct score_entry *newscores
= (struct score_entry *) realloc (*scores,
sizeof (struct score_entry) * ((*count) + 1));
if (!newscores)
return -1;
newscores[*count].score = newscore;
newscores[*count].username = username;
newscores[*count].data = newdata;
struct score_entry *newscores = *scores;
if (*count == *size)
{
ptrdiff_t newsize = *size;
if (newsize <= 0)
newsize = 1;
else if (newsize <= MAX_SCORES / 2)
newsize *= 2;
else if (newsize < MAX_SCORES)
newsize = MAX_SCORES;
else
{
errno = ENOMEM;
return -1;
}
newscores = realloc (newscores, sizeof *newscores * newsize);
if (!newscores)
return -1;
*scores = newscores;
*size = newsize;
}
newscores[*count] = *newscore;
(*count) += 1;
*scores = newscores;
return 0;
}
static void
sort_scores (struct score_entry *scores, int count, int reverse)
sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse)
{
qsort (scores, count, sizeof (struct score_entry),
reverse ? score_compare_reverse : score_compare);
qsort (scores, count, sizeof *scores,
reverse ? score_compare_reverse : score_compare);
}
static int
write_scores (const char *filename, const struct score_entry *scores, int count)
write_scores (const char *filename, const struct score_entry *scores,
ptrdiff_t count)
{
int fd;
FILE *f;
int i;
ptrdiff_t i;
char *tempfile = malloc (strlen (filename) + strlen (".tempXXXXXX") + 1);
if (!tempfile)
return -1;
@ -403,8 +452,9 @@ write_scores (const char *filename, const struct score_entry *scores, int count)
if (! f)
return -1;
for (i = 0; i < count; i++)
if (fprintf (f, "%ld %s %s\n", scores[i].score, scores[i].username,
scores[i].data) < 0)
if (fprintf (f, "%"PRIdMAX" %s %s\n",
scores[i].score, scores[i].username, scores[i].data)
< 0)
return -1;
fclose (f);
if (rename (tempfile, filename) < 0)
@ -459,10 +509,11 @@ static int
unlock_file (const char *filename, void *state)
{
char *lockpath = (char *) state;
int ret = unlink (lockpath);
int saved_errno = errno;
int ret = unlink (lockpath);
int unlink_errno = errno;
free (lockpath);
errno = saved_errno;
errno = ret < 0 ? unlink_errno : saved_errno;
return ret;
}