Jon Steinhart <jon(_at_)fourwinds(_dot_)com> wrote on May 13, 2005:
Jon Steinhart <jon(_at_)fourwinds(_dot_)com> wrote on May 13, 2005:
Saw this while looking for something else.
m_chkids() forks a child process to run context_save() if the
uid is not the same as the euid. But, it ends up running as
if the uid and euid are the same if the fork() fails. Seems
to me that this should be an error. I realize that it will
probably result in later errors from being unable to access
the files, but those will be confusing since they won't indicate
the real problem.
Opinions?
You shouldn't be making mh commands setuid, so the situation is
unlikely to arise. This probably isn't worth fixing, except as part
of a complete revamp of core code.
So give me a clue here. Why shouldn't they be made setuid? Someone
obviously thought enough about this to put this code there in the
first place. If running setuid is a bad thing and shouldn't be done
would it be acceptable to just remove this whole piece of code?
It was probably put into the library for generality.
I perhaps overstated things. Better would be that, with only one
exception, I cannot think of a reason to run the mh commands setuid.
The one exception is "slocal", which might be called from sendmail or
similar MTA to deliver, and might inherit root euid. My memory of
the code is that slocal checks for this condition, and does a
setuid() to the actual recipient quite early in the code, so it
should not be setuid by the time it is looking at context.
What can go wrong if euid != ruid, and that fork() fails?
The possibilities I see are:
1: the euid in use does not have permission to update the
context. In this case the command fails anyway.
2: the update succeeds, rewriting an existing file. This does
not seem to pose a problem.
3: the update succeeds creating or re-creating the file. The data
is still saved, but the created file has the wrong ownership.
The third case is the more questionable. Which is better -- save the
data under the wrong ownership, or lose the data? I suspect it isn't
very important, especially since it probably won't happen anyway.
Maybe there is a security risk here. However, I would think that
the risk is in allowing these commands (except slocal) to run setuid
in the first place.
In other words, I don't see a serious problem with just leaving the
code as it is for the present. Or perhaps add a comment that this
should be reviewed if/when the code is revamped.
Feel free to disagree (that applies to anybody).
-NWR
_______________________________________________
Nmh-workers mailing list
Nmh-workers(_at_)nongnu(_dot_)org
http://lists.nongnu.org/mailman/listinfo/nmh-workers