lkml.org 
[lkml]   [2004]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: exec: atomic MAY_EXEC check and SUID/SGID handling
    On Thu, Sep 02, 2004 at 01:31:09PM -0700, Chris Wright wrote:
    > * Andrey Savochkin (saw@saw.sw.com.sg) wrote:
    > > There is a time window between permission(MAY_EXEC) check in
    > > open_exec() and S_ISUID check plus bprm->e_uid setting in prepare_binprm().
    > > And S_ISUID is checked and bprm->e_uid is copied from the inode without
    > > any serialization with attribute updates.
    > >
    > > That means that some executable may have permissions
    > > -rwxr-xr-x root disk /bin/file
    > > at the moment of MAY_EXEC check and
    > > -rwsr-x--- root disk /bin/file
    > > at the moment of S_ISUID check, providing lucky users starting /bin/file at
    > > the moment of permission change with a setuid-root program.
    > >
    > > It's arguable whether it's a big security issue, but certainly such behavior
    > > is not what administrators may expect.
    >
    > If you can find a way for a user to exploit this it's an issue. Looks

    Exploiting it requires waiting for the administrator to change file
    permissions... May be, some social engineering.
    But THERE IS a race, which may result in user having more permissions than
    he is expected to have.
    I'm not comfortable living with such a race.

    Instead of
    inode->i_mode = attr->ia_mode;
    we can write inode_setattr() as
    inode->i_mode |= 06777;
    inode->i_mode &= attr->ia_mode;

    Will it be easily exploitable? I guess, no.
    Will I be comfortable if the code is vulnerable in this way? No.

    > like it's not, and doesn't warrant such a big change as your patch.
    > The fact that you introduce a new field and then almost always supply it
    > with NULL is a clue that it's not the right direction IMO. Something
    > simple (as you mentioned) that grabs i_sem and rechecks during suid
    > setup in binprm_prepare is sufficient. Worth it? Guess I'm not
    > convinced.

    I explained my arguments against re-checking permissions:
    - the locking convention where ->permission() method may be called with or
    without i_sem doesn't look suberb;
    - it's better to avoid calling permission() with the same arguments for
    the second time, especially if it does something complicated in
    security_inode_permission(), with ACLs or in case of a remote filesystem.

    Andrey
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:05    [W:3.797 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site