On 09/19/2016 10:32 AM, Jann Horn wrote:
> On Mon, Sep 19, 2016 at 09:01:19AM -0400, Stephen Smalley wrote:
>> On 09/18/2016 11:05 AM, Jann Horn wrote:
>>> This adds a new ptrace_may_access_noncurrent() method that
>>> uses the supplied credentials instead of those of the
>>> current task. (However, the current task may still be
>>> inspected for auditing purposes, e.g. by the Smack LSM.)
>>>
>>> procfs used the caller's creds for a few ptrace_may_access()
>>> checks at read() time, which made a confused deputy attack
>>> by passing an FD to a procfs file to a setuid program
>>> possible. Therefore, the following was a local userspace
>>> ASLR bypass:
> [...]
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index 13185a6..f9a0be7 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -2133,10 +2133,10 @@ static int selinux_binder_transfer_file(struct task_struct *from,
>>> }
>>>
>>> static int selinux_ptrace_access_check(struct task_struct *child,
>>> - unsigned int mode)
>>> + unsigned int mode, const struct cred *cred)
>>> {
>>> if (mode & PTRACE_MODE_READ) {
>>> - u32 sid = current_sid();
>>> + u32 sid = cred_sid(cred);
>>> u32 csid = task_sid(child);
>>> return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL);
>>> }
>>
>> For consistency, don't you also need to change the next line of code to
>> use cred_has_perm() rather than current_has_perm()?
>
> Oh, right.
>
> How about something like this?
>
> u32 sid = cred_sid(cred);
> u32 csid = task_sid(child);
> if (mode & PTRACE_MODE_READ)
> return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL);
> else
> return avc_has_perm(sid, csid, SECCLASS_PROCESS, PROCESS__PTRACE, NULL);
>
> The current code looks as if the PTRACE_MODE_READ and PTRACE_MODE_ATTACH cases
> behave very differently, which they actually don't. And to be able to use
> cred_has_perm() here, it would be necessary to explicitly grab and release
> an RCU read lock or so (to prevent the child creds from going away).
>
Sure, that's fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
