Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180720154723.5rgdhg2n5nj5bgce@jwilk.net>
Date: Fri, 20 Jul 2018 17:47:24 +0200
From: Jakub Wilk <jwilk@...lk.net>
To: oss-security@...ts.openwall.com
Subject: Re: accountsservice: insufficient path check in
 user_change_icon_file_authorized_cb()

* Matthias Gerstner <mgerstner@...e.de>, 2018-07-02, 16:38:
>>>I think the easiest way to fix this is to normalize the user supplied 
>>>filename e.g. using realpath()
>>
>>Using realpath(3) for access control is almost always a mistake: this 
>>function expands symlinks, including attacker-controlled symlinks.
>
>can you elaborate what your main worry of using realpath is in this 
>context?

AIUI, in your original patch, canonicalized path was used for prefix 
check, but then the orignal was stored. If you used realpath() for 
canonicalization, the attacker could make a symlink that points to 
/usr/share/icons/moo.png, so that the check passes, and then switch 
the symlink to something else.

But in the patch that went upstream[0], it's the canonicalized path that 
is stored, which is probably a good idea anyway.

Another problem with realpath(), unrelated to symlinks, is that if it's 
run as root, it could reveal to the attacker whether an 
otherwise-inaccessible directory exists. For example, 
realpath("/home/bob/foobar/../../../usr/share/icons/moo.png", ...) would 
succeed iff /home/bob/foobar/ existed.


[0] https://cgit.freedesktop.org/accountsservice/commit/?id=f9abd359f71a5bce421b9ae23432f539a067847a

-- 
Jakub Wilk

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.