Bug 270404 - comsat is willing to try to read and display any file
Summary: comsat is willing to try to read and display any file
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Many People
Assignee: Ed Maste
URL: https://reviews.freebsd.org/D47823
Keywords: security
Depends on:
Blocks:
 
Reported: 2023-03-22 14:18 UTC by Robert Morris
Modified: 2025-03-02 11:59 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2023-03-22 14:18:02 UTC
If comsat is enabled in /etc/inetd.conf, and user is logged in and
accepting notifications, then sending comsat a UDP packet like

  user@0:../../etc/remote

will cause comsat to try to open /etc/remote (really
/var/mail/../../etc/remote) and display it on user's terminal.
Comment 1 Robert Clausecker freebsd_committer freebsd_triage 2023-03-22 17:17:51 UTC
I'm sorry, I think I did something wrong with bugzilla and made the bug invisible.
Comment 2 Kyle Evans freebsd_committer freebsd_triage 2023-03-22 18:29:21 UTC
Open the bug back up to reporter/cc list, secteam can open it up further if they want to.
Comment 3 Ed Maste freebsd_committer freebsd_triage 2024-11-27 20:36:27 UTC
You don't even need to make use of .. traversal; user@0:/etc/remote will also work as comsat is prepared to handle absolute paths (for whatever reason).

That said, after a brief investigation I think there's no significant security concern here. Comsat generally does setuid to the appropriate user before attempting to read a file, so this issue alone shouldn't allow access to arbitrary files that the user could not already access anyway.

The setuid call does only happen if getpwnam returns an entry for the user though:

        /* Set effective uid to user in case mail drop is on nfs */
        if ((p = getpwnam(user)) != NULL)
                (void) setuid(p->pw_uid);

It seems that if the user isn't known or the setuid fails we'd be better to just return, e.g.

        if ((p = getpwnam(user)) == NULL)
                return;
        if (setuid(p->pw_uid) != 0)
                return;

inetd.conf contains the note:

# run comsat as root to be able to print partial mailbox contents w/ biff,
# or use the safer tty:tty to just print that new mail has been received.

If running as tty the change proposed above would have comsat return from jkfprintf due to the setuid call failing rather than because the fopen failed, but that's OK.

Of course even better would be to just remove comsat all together, but probably as a later step.
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-11-28 13:05:23 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=062b69ba045dc0fef3d9b8d73365d2798c05a480

commit 062b69ba045dc0fef3d9b8d73365d2798c05a480
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2024-11-27 20:36:46 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-11-28 13:04:10 +0000

    comsat: Improve use of setuid()

    Just return from jkfprintf if either (a) user lookup fails (that is,
    getpwnam fails) or (b) setuid() to the user's uid fails.  If comsat is
    invoked from inetd using the default of tty:tty we will now return due
    to setuid() failing rather than fopen() failing.

    PR:             270404
    Reviewed by:    kevans
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D47823

 libexec/comsat/comsat.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-12-01 20:31:05 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d4dd9e22c13896e6b5e2a6fc78dad4f8496cc14d

commit d4dd9e22c13896e6b5e2a6fc78dad4f8496cc14d
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2024-11-28 16:54:48 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-12-01 20:29:02 +0000

    comsat: Use initgroups and setgid not just setuid

    PR:             270404
    Reviewed by:    jlduran
    Obtained from:  NetBSD
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D47828

 libexec/comsat/comsat.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Comment 6 Ed Maste freebsd_committer freebsd_triage 2024-12-01 20:47:45 UTC
We should probably use NetBSD's approach - ignore the path in a comsat message, unless it is the system path.

commit 4c4a0e2830a28ffdc555143b8ea45f2b70397aa4
Author: atatat <atatat@NetBSD.org>
Date:   Fri Mar 16 21:39:08 2001 +0000

    Use strtol() instead of atoi() for reading the number out of the
    datagram that we received, which leads to easier support for
    (ignoring) the procmail messages that specify the folder to which
    the message was delivered.
    
    When reading the mailbox, if we encounter a "From " line, we should
    exit().  This can occur if there are a lot of rapidly arriving, yet
    short messages.

Comment from NetBSD comsat.c:
               /*
                * Procmail sends messages to comsat with a trailing colon
                * and a pathname to the folder where the new message was
                * deposited.  Since we can't reliably open only regular
                * files, we need to ignore these.  With one exception:
                * if it mentions the user's system mailbox.
                */
Comment 7 commit-hook freebsd_committer freebsd_triage 2024-12-02 21:11:49 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=957f7a2a58e550bd31d8ebec67f99d19087746a2

commit 957f7a2a58e550bd31d8ebec67f99d19087746a2
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2024-11-27 20:36:46 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-12-02 21:10:31 +0000

    comsat: Improve use of setuid()

    Just return from jkfprintf if either (a) user lookup fails (that is,
    getpwnam fails) or (b) setuid() to the user's uid fails.  If comsat is
    invoked from inetd using the default of tty:tty we will now return due
    to setuid() failing rather than fopen() failing.

    PR:             270404
    Reviewed by:    kevans
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D47823

    (cherry picked from commit 062b69ba045dc0fef3d9b8d73365d2798c05a480)

 libexec/comsat/comsat.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-12-04 18:39:32 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bb9678f1ff6881b036220045adb58047332cfb0d

commit bb9678f1ff6881b036220045adb58047332cfb0d
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2024-11-28 16:54:48 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-12-04 18:38:31 +0000

    comsat: Use initgroups and setgid not just setuid

    PR:             270404
    Reviewed by:    jlduran
    Obtained from:  NetBSD
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D47828

    (cherry picked from commit d4dd9e22c13896e6b5e2a6fc78dad4f8496cc14d)

 libexec/comsat/comsat.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Comment 9 Ed Maste freebsd_committer freebsd_triage 2024-12-06 18:33:20 UTC
Still planning to fix the actual issue here (that comsat is willing to try to open an arbitrary file)