Date: Fri, 14 Sep 2001 12:26:14 +0300 From: Ruslan Ermilov <ru@FreeBSD.ORG> To: "Andrew R. Reiter" <arr@watson.org> Cc: freebsd-audit@FreeBSD.ORG Subject: Re: dungeon master patch Message-ID: <20010914122614.A82568@sunbay.com> In-Reply-To: <Pine.NEB.3.96L.1010914000517.11262A-200000@fledge.watson.org>; from arr@watson.org on Fri, Sep 14, 2001 at 12:08:35AM -0400 References: <Pine.NEB.3.96L.1010914000517.11262A-200000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Sep 14, 2001 at 12:08:35AM -0400, Andrew R. Reiter wrote:
> hey,
>
> I just started to go through -current (seemingly on default) s{g,u}id bins
> and their source for security vulns. I found a few definet coding
> problems in dungeon master (setgid games ;-)), however, since you can't
> specify the config file, they are probably non-exploitable. but, hey,
> it's being installed setgid (even if it is games), might as well use good
> coding practice.
>
> the patch is attached, and also can be found at:
>
At a quick glance.
> --- dm.c.orig Thu Sep 13 22:44:25 2001
> +++ dm.c Thu Sep 13 23:02:43 2001
> @@ -111,15 +111,16 @@
> play(args)
> char **args;
> {
> - char pbuf[MAXPATHLEN];
> + char pbuf[MAXPATHLEN+1];
>
Err, the correct value is MAXPATHLEN. Search intro(2) for more details.
> - if (sizeof(_PATH_HIDE) + strlen(game) > sizeof(pbuf)) {
> + if (sizeof(_PATH_HIDE) + strlen(game) + 1 > sizeof(pbuf)) {
Err, sizeof("foo") is 4, not 3. Hence the space for final '\0' is already
accumulated with sizeof(_PATH_HIDE).
> (void)fprintf(stderr, "dm: %s/%s: %s\n", _PATH_HIDE, game,
> strerror(ENAMETOOLONG));
> exit(1);
> }
> - (void)strcpy(pbuf, _PATH_HIDE);
> - (void)strcpy(pbuf + sizeof(_PATH_HIDE) - 1, game);
> + bzero((void *)&pbuf, MAXPATHLEN+1);
> + strlcpy(pbuf, _PATH_HIDE, sizeof(pbuf));
> + strlcat(pbuf+strlen(_PATH_HIDE), game, sizeof(pbuf)-strlen(_PATH_HIDE));
There was nothing wrong with this.
> if (priority > 0) /* < 0 requires root */
> (void)setpriority(PRIO_PROCESS, 0, priority);
> execv(pbuf, args);
> @@ -135,30 +136,37 @@
> read_config()
> {
> FILE *cfp;
> - char lbuf[BUFSIZ], f1[40], f2[40], f3[40], f4[40], f5[40];
> + char lbuf[BUFSIZ+1], f1[40], f2[40], f3[40], f4[40], f5[40];
>
> if (!(cfp = fopen(_PATH_CONFIG, "r")))
> return;
> - while (fgets(lbuf, sizeof(lbuf), cfp))
> + while (fgets(lbuf, sizeof(lbuf)-1, cfp)) {
The fgets() function reads at most one less than the number of characters
specified by size from the given stream and stores them in the string
str.
> + bzero(&f1, sizeof(f1));
> + bzero(&f2, sizeof(f2));
> + bzero(&f3, sizeof(f3));
> + bzero(&f4, sizeof(f4));
> + bzero(&f5, sizeof(f5));
Umm, sscanf(3) places a trailing NUL character:
: s Matches a sequence of non-white-space characters; the next pointer
: must be a pointer to char, and the array must be large enough to
: accept all the sequence and the terminating NUL character. The
: input string stops at white space or at the maximum field width,
: whichever occurs first.
> switch(*lbuf) {
> case 'b': /* badtty */
> - if (sscanf(lbuf, "%s%s", f1, f2) != 2 ||
> + if (sscanf(lbuf, "%39s%39s", f1, f2) != 2 ||
Use "%.s" and "sizeof(f1) - 1", don't hardcode.
> strcasecmp(f1, "badtty"))
> break;
> c_tty(f2);
> break;
> case 'g': /* game */
> - if (sscanf(lbuf, "%s%s%s%s%s",
> + if (sscanf(lbuf, "%39s%39s%39s%39s%39s",
> f1, f2, f3, f4, f5) != 5 || strcasecmp(f1, "game"))
> break;
> c_game(f2, f3, f4, f5);
> break;
> case 't': /* time */
> - if (sscanf(lbuf, "%s%s%s%s", f1, f2, f3, f4) != 4 ||
> - strcasecmp(f1, "time"))
> + if (sscanf(lbuf, "%39s%39s%39s%39s",
> + f1, f2, f3, f4) != 4 || strcasecmp(f1, "time"))
> break;
> c_day(f2, f3, f4);
> }
> + bzero(&lbuf, sizeof(lbuf));
> + }
> (void)fclose(cfp);
> }
>
--
Ruslan Ermilov Oracle Developer/DBA,
ru@sunbay.com Sunbay Software AG,
ru@FreeBSD.org FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010914122614.A82568>