[Freeciv-Dev] (PR#39901) Patch: Fix permission on open() call
http://bugs.freeciv.org/Ticket/Display.html?id=39901 > Please use the email interface, your comments were not sent to the list. > Not really, it's more of a security thing. > > Quote from one of our kernel guys: "The problem is that without a mode > being passed, the kernel uses whatever the stack contents are. And yes, > its conceivable the stack contents could create a world writable setuid > file which cannot ever be the intended operation." Speaking as a long-time Internet security guy, that sounds like a serious userland/kernel interface implementation bug First of all, according to the documentation, the mask is optional -- you really need to use varargs here, that's what the "..." means in the documentation prototype. Secondly, according to the documentation, the mask is AND'd with the current umask. There *MUST NOT* be any way for AND to set new bits! I'll pass this along to the Linux NFS kernel maintainers when I see them on Wednesday ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39901) Patch: Fix permission on open() call
http://bugs.freeciv.org/Ticket/Display.html?id=39901 >
William Allen Simpson wrote:
> Is this a Fedora only kind of thing?
>
So, I did a bit of Googling, and there are a number of other reports about
Fedora and this new requirement As a long-time BSD contributor, one of
the things that annoys me about certain Linux practices is the failure to
follow the documentation (in this case, "may" really should be optional).
Never-the-less, I tested, and there is another bug on the same line. The
server overwrites parts of the client generated log. It needs O_APPEND.
There is also a comment that isn't implemented. I added FIXME, until the
issue is better understood.
In fact, this whole logging section seems incompletely implemented. Why is
the debug level for the server always set to 3, no matter what the level on
the client?
Anyway, since this actually has a bug, I'll commit immediately. Thanks!
Committed S2_1 revision 14050.
Committed S2_2 revision 14051.
Committed trunk revision 14052.
Index: client/connectdlg_common.c
===
--- client/connectdlg_common.c (revision 14049)
+++ client/connectdlg_common.c (working copy)
@@ -250,9 +250,9 @@
fclose(stdout);
fclose(stderr);
-/* include the port to avoid duplication */
+/* FIXME: include the port to avoid duplication? */
if (logfile) {
- fd = open(logfile, O_WRONLY | O_CREAT);
+ fd = open(logfile, O_WRONLY | O_CREAT | O_APPEND, 0644);
if (fd != 1) {
dup2(fd, 1);
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39901) Patch: Fix permission on open() call
http://bugs.freeciv.org/Ticket/Display.html?id=39901 > > open() with O_CREAT in its second argument needs to have a third argument > That's not accurate. Only when you wish to restrict the file mode mask. Without looking hard at this code, I don't see any reason we'd want to restrict the mask. That's not traditional. Is this a Fedora only kind of thing? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39901) Patch: Fix permission on open() call
http://bugs.freeciv.org/Ticket/Display.html?id=39901 >
open() with O_CREAT in its second argument needs to have a third argument
Thanks,
Brian Pepple
Index: client/connectdlg_common.c
===
--- client/connectdlg_common.c (revision 14044)
+++ client/connectdlg_common.c (working copy)
@@ -245,7 +245,7 @@
/* include the port to avoid duplication */
if (logfile) {
- fd = open(logfile, O_WRONLY | O_CREAT);
+ fd = open(logfile, O_WRONLY | O_CREAT, 0644);
if (fd != 1) {
dup2(fd, 1);
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
