Thank you, egmont, for your time answering my question. All you say about
vcs* sounds reasonable, unfortunately according to the code, the tty owner
is the problem.

fstat (console_fd, &st) < 0 || st.st_uid != uid

As far as I understand the c code here, you compare the user id with the id
of the tty owner. This happens in the very beginning of the main function,
and then in

case CONSOLE_SAVE

if (seteuid (euid) < 0
                || lseek (vcsa_fd, 0, 0) != 0
                || fstat (console_fd, &st) < 0 || st.st_uid != uid
                || read (vcsa_fd, buffer, buffer_size) != buffer_size
                || fstat (console_fd, &st) < 0 || st.st_uid != uid)

and then in

case CONSOLE_RESTORE

if (seteuid (euid) >= 0
                && lseek (vcsa_fd, 0, 0) == 0 && fstat (console_fd, &st) >=
0 && st.st_uid == uid)

Yes, you set the effective user id (in case when cons.server has that
sticky bit and is owned by vcsa, it is the vcsa user). You need it to be
able to

lseek (vcsa_fd, 0, 0) and  read (vcsa_fd, buffer, buffer_size)

But then you compare the  owner id of the tty with the real user id (in my
example it is the root user), the comparison fails and the root user sees
the blank screen in one case, and has to press the `anyKey` in the other as
described earlier.

Disregarding of what was the intention,  disregarding of what you were
trying to achieve and what security hole to close, do you think, that sort
of comparison is valid here?

Thank you.

--
Konstantin

On 10 March 2017 at 02:30, Egmont Koblinger <egm...@gmail.com> wrote:

> Hi,
>
> cons.saver, as you apparently know this, is the helper binary responsible
> for restoring the contents of the Linux console when you quit mc or press
> Ctrl+O. A helper is required since the Linux console does not have an
> "alternate screen" that graphical terminal emulators have.
>
> In order to be able to do this, it needs read/write access to /dev/vcsa*.
>
> When you log in on the console, the corresponding /dev/tty* becomes owned
> by you but /dev/vcsa* don't. I believe the reason behind it is that there
> is a way to revoke the tty from you, but there is no way to revoke the vcsa
> access. That is, when you log out, you might keep a background process
> running which still has access to it via a previously opened file
> descriptor, and subsequently as someone else logs in, you could spy on the
> console's contents.
>
> As such, since /dev/vcsa* is not owned by the desired user, cons.saver
> needs to be setgid tty (or setuid root).
>
> Setuid/setgid apps must have all these kinds of precautions that you're
> asking about, they need to duplicate the permission checks because they are
> not being run as the actual real user. It's crucial that someone not
> actually sitting in front of the tty cannot trick cons.saver into tampering
> with the tty's contents.
>
> Hope this explains the situation.
>
> I'm not sure why something is checked twice, but it can easily be in order
> to avoid a race condition (or could easily be a harmless bug as well).
>
> egmont
>
>
> On Fri, Mar 10, 2017 at 1:06 AM, Key Offecka <key.offe...@gmail.com>
> wrote:
>
>> Hi,
>>
>> I am looking at the
>>
>> main (int argc, char **argv)
>>
>> function in
>>
>> src/consaver/cons.saver.c
>>
>> There are calls like
>>
>> st.st_uid != uid
>>
>> fstat (console_fd, &st) >= 0 && st.st_uid == uid
>>
>> fstat (console_fd, &st) < 0 || st.st_uid != uid
>>
>> The last one is especially strange taking into account that it appears
>> twice
>>
>> if (seteuid (euid) < 0
>>                 || lseek (vcsa_fd, 0, 0) != 0
>>                 || fstat (console_fd, &st) < 0 || st.st_uid != uid
>>                 || read (vcsa_fd, buffer, buffer_size) != buffer_size
>>                 || fstat (console_fd, &st) < 0 || st.st_uid != uid)
>>
>> This all is taken from the commit e9fd11bfcd1dab97e3ba423bcfb8b6ca1088b11c
>> which is the latest at this moment
>>
>> It looks to me MC tries inventing its own permission scheme rather than
>> relying on the system set up.
>> Consider there is a user in the system who is allowed to read/write and
>> to do whatever they want with vcs, tty and with whatever files else you may
>> only wish. root is one obvious candidate but nothing restricts us to set up
>> another user taking advantaged of  all those system security facilities.
>> There is a traditional UNIX permission scheme, SeLinux may be involved if
>> needed. And now comes MC, and introduces a hardcoded/unconfigurable/solid
>> as a stone requirement for the current user to be the owner of the files.
>> Why so?
>>
>> I believe there is case and that code is called to cover it. But
>> unfortunately I do not see the reason. And this is my question, I would
>> appreciate if anybody could explain what security issue was addressed here?
>> In my particular case this code introduces an inconvenience, so I just
>> removed it and feel total happy without it. But still am a little bit
>> concerned about possible consequences which I do now understand at the
>> moment.
>>
>> My case I mentioned above is as follows:
>>
>> Log into, say, tty3 as a normal user, say `echo`. The tty3 ownership
>> changes, and the `echo` user becomes the owner of tty3 which sounds
>> reasonable.
>> Now sudo as another user who has all access permissions to tty and vcs,
>> In my case this is root.
>> Press Control+O, MC screws up the background shell, the root user sees
>> the blank screen rather than previously executed commands and MC starts
>> thinking the terminal is dumb asking to press any key after executing
>> commands. And this happens for the root user! MC overwrote the root
>> privileges! Does it sound reasonable to you?
>>
>> Any explanations are welcome.
>> Thank you.
>>
>> --
>> Konstantin I.,
>>
>>
>> _______________________________________________
>> mc-devel mailing list
>> https://mail.gnome.org/mailman/listinfo/mc-devel
>>
>>
>
_______________________________________________
mc-devel mailing list
https://mail.gnome.org/mailman/listinfo/mc-devel

Reply via email to