tiny tetris patch
Hi all,
As tetris is one of my preferred game :-) ... just did wrapper around
setegid in same manner than xmalloc and such. If it can find any use ...
Thanks.
Index: scores.c
===
RCS file: /cvs/src/games/tetris/scores.c,v
retrieving revision 1.12
diff -u -p -r1.12 scores.c
--- scores.c16 Nov 2014 04:49:49 - 1.12
+++ scores.c15 Mar 2015 09:08:46 -
@@ -111,11 +111,11 @@ getscores(FILE **fpp)
human = reading;
lck = LOCK_SH;
}
- setegid(egid);
+ xsetegid(egid);
mask = umask(S_IWOTH);
sd = open(_PATH_SCOREFILE, mint, 0666);
(void)umask(mask);
- setegid(gid);
+ xsetegid(gid);
if (sd 0) {
if (fpp == NULL) {
nscores = 0;
@@ -123,10 +123,10 @@ getscores(FILE **fpp)
}
err(1, cannot open %s for %s, _PATH_SCOREFILE, human);
}
- setegid(egid);
+ xsetegid(egid);
if ((sf = fdopen(sd, mstr)) == NULL)
err(1, cannot fdopen %s for %s, _PATH_SCOREFILE, human);
- setegid(gid);
+ xsetegid(gid);
/*
* Grab a lock.
Index: tetris.c
===
RCS file: /cvs/src/games/tetris/tetris.c,v
retrieving revision 1.25
diff -u -p -r1.25 tetris.c
--- tetris.c16 Nov 2014 04:49:49 - 1.25
+++ tetris.c15 Mar 2015 09:08:46 -
@@ -161,7 +161,7 @@ main(int argc, char *argv[])
gid = getgid();
egid = getegid();
- setegid(gid);
+ xsetegid(gid);
classic = showpreview = 0;
while ((ch = getopt(argc, argv, ck:l:ps)) != -1)
@@ -363,4 +363,11 @@ usage(void)
{
(void)fprintf(stderr, usage: tetris [-cps] [-k keys] [-l level]\n);
exit(1);
+}
+
+void
+xsetegid(gid_t gid)
+{
+if (setegid(gid) == -1)
+err(1, setegid() failed);
}
Index: tetris.h
===
RCS file: /cvs/src/games/tetris/tetris.h,v
retrieving revision 1.10
diff -u -p -r1.10 tetris.h
--- tetris.h10 Aug 2008 12:23:25 - 1.10
+++ tetris.h15 Mar 2015 09:08:46 -
@@ -176,3 +176,4 @@ extern int classic;
intfits_in(const struct shape *, int);
void place(const struct shape *, int, int);
void stop(char *);
+void xsetegid(gid_t);
Re: tiny tetris patch
Hi David,
David CARLIER wrote on Sun, Mar 15, 2015 at 09:09:25AM +:
As tetris is one of my preferred game :-) ... just did wrapper around
setegid in same manner than xmalloc and such. If it can find any use ...
This doesn't make sense to me.
The global variables gid and egid are only set at one place;
actually, it's visible in your patch itself in tetris.c.
So we know both are always the process's real, effective, or saved GID.
Consequently, setegid() cannot fail, and there is no need to check.
Even if it could fail, error handling is already in place:
err(1, cannot open %s for %s, _PATH_SCOREFILE, human);
err(1, cannot fdopen %s for %s, _PATH_SCOREFILE, human);
That's also visible in your patch.
To summarize, there is no need to change anything.
Yours,
Ingo
Index: scores.c
===
RCS file: /cvs/src/games/tetris/scores.c,v
retrieving revision 1.12
diff -u -p -r1.12 scores.c
--- scores.c 16 Nov 2014 04:49:49 - 1.12
+++ scores.c 15 Mar 2015 09:08:46 -
@@ -111,11 +111,11 @@ getscores(FILE **fpp)
human = reading;
lck = LOCK_SH;
}
- setegid(egid);
+ xsetegid(egid);
mask = umask(S_IWOTH);
sd = open(_PATH_SCOREFILE, mint, 0666);
(void)umask(mask);
- setegid(gid);
+ xsetegid(gid);
if (sd 0) {
if (fpp == NULL) {
nscores = 0;
@@ -123,10 +123,10 @@ getscores(FILE **fpp)
}
err(1, cannot open %s for %s, _PATH_SCOREFILE, human);
}
- setegid(egid);
+ xsetegid(egid);
if ((sf = fdopen(sd, mstr)) == NULL)
err(1, cannot fdopen %s for %s, _PATH_SCOREFILE, human);
- setegid(gid);
+ xsetegid(gid);
/*
* Grab a lock.
Index: tetris.c
===
RCS file: /cvs/src/games/tetris/tetris.c,v
retrieving revision 1.25
diff -u -p -r1.25 tetris.c
--- tetris.c 16 Nov 2014 04:49:49 - 1.25
+++ tetris.c 15 Mar 2015 09:08:46 -
@@ -161,7 +161,7 @@ main(int argc, char *argv[])
gid = getgid();
egid = getegid();
- setegid(gid);
+ xsetegid(gid);
classic = showpreview = 0;
while ((ch = getopt(argc, argv, ck:l:ps)) != -1)
@@ -363,4 +363,11 @@ usage(void)
{
(void)fprintf(stderr, usage: tetris [-cps] [-k keys] [-l level]\n);
exit(1);
+}
+
+void
+xsetegid(gid_t gid)
+{
+if (setegid(gid) == -1)
+err(1, setegid() failed);
}
Index: tetris.h
===
RCS file: /cvs/src/games/tetris/tetris.h,v
retrieving revision 1.10
diff -u -p -r1.10 tetris.h
--- tetris.h 10 Aug 2008 12:23:25 - 1.10
+++ tetris.h 15 Mar 2015 09:08:46 -
@@ -176,3 +176,4 @@ extern intclassic;
int fits_in(const struct shape *, int);
void place(const struct shape *, int, int);
void stop(char *);
+void xsetegid(gid_t);
Re: tiny tetris patch
On Sun, Mar 15, 2015 at 1:21 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: One day, it would be nice if /var cannot be filled up in a hostile fashion... slightly off-topic, but I routinely make /var and /var/log separate filesystems (especially on Internet-facing hosts). this might be worth considering as a default behavior for the installer. it doesn't completely fix the problem but it at least mitigates one of the more frequent causes. -ken
Re: tiny tetris patch
The global variables gid and egid are only set at one place; actually, it's visible in your patch itself in tetris.c. So we know both are always the process's real, effective, or saved GID. Consequently, setegid() cannot fail, and there is no need to check. Yes. Long term, I would like to see the games stop being setgid as a way to write to /var. One day, it would be nice if /var cannot be filled up in a hostile fashion...
Re: tiny tetris patch
On Sun, Mar 15, 2015 at 1:21 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: One day, it would be nice if /var cannot be filled up in a hostile fashion... slightly off-topic, but I routinely make /var and /var/log separate filesystems (especially on Internet-facing hosts). this might be worth considering as a default behavior for the installer. it doesn't completely fix the problem but it at least mitigates one of the more frequent causes. With only 14 partitions available, that is a bit of a loss. And unfortunately /var/log is not the only attack surface.
