Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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

Your e-mail address:

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

Powered by Openwall GNU/*/Linux - Powered by OpenVZ