linux-kernel - Re: [GIT PULL] Fix for Integrity subsystem null pointer deref

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 29 Oct 2014 14:29:49 -0400
From:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	LSM List <linux-security-module@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"security@...nel.org" <security@...nel.org>,
	James Morris <jmorris@...ei.org>
Subject: Re: [GIT PULL] Fix for Integrity subsystem null pointer deref

On Wed, 2014-10-29 at 09:23 -0700, Andy Lutomirski wrote: 
> On Oct 29, 2014 6:00 AM, "Mimi Zohar" <zohar@...ux.vnet.ibm.com> wrote:
> >
> > On Tue, 2014-10-28 at 22:08 -0700, Andy Lutomirski wrote:
> > > On Tue, Oct 28, 2014 at 8:55 PM, James Morris <jmorris@...ei.org> wrote:
> > > > These changes fix a bug in xattr handling, where the evm and ima
> > > > inode_setxattr() functions do not check for empty xattrs being passed from
> > > > userspace (leading to user-triggerable null pointer dereferences).
> > > >
> > > > Please pull.
> > > >
> > > >
> > > > The following changes since commit 9f76628da20f96a179ca62b504886f99ecc29223:
> > > >
> > > >   Merge branch 'for-3.18' of git://linux-nfs.org/~bfields/linux (2014-10-28 13:32:06 -0700)
> > > >
> > > > are available in the git repository at:
> > > >
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus
> > > >
> > > > Dmitry Kasatkin (2):
> > > >       ima: check xattr value length and type in the ima_inode_setxattr()
> > >
> > > I haven't read this one, but:
> > >
> > > >       evm: check xattr value length and type in evm_inode_setxattr()
> > >
> > > const struct evm_ima_xattr_data *xattr_data = xattr_value;
> > > - if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0)
> > > - && (xattr_data->type == EVM_XATTR_HMAC))
> > > - return -EPERM;
> > > + if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
> > > + if (!xattr_value_len)
> > > + return -EINVAL;
> > > + if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
> > > + return -EPERM;
> > > + }
> > >
> > > Huh?  (Sorry about severe whitespace damage.)
> > >
> > > Shouldn't there be something like if (xattr_value_len < sizeof(struct
> > > evm_ima_xattr_data)) return -EINVAL?
> >
> > Prior to commit 2fb1c9a "evm: prohibit userspace writing 'security.evm'
> > HMAC value", a process with CAP_SYS_ADMIN could write either an HMAC or
> > signature. As the HMAC key should only be known to the kernel, only
> > signatures are now allowed.  Instead of "struct evm_ima_xattr_data", the
> > code should reflect this change and use "struct signature_v2_hdr".
> > We'll clean up this code for the next release.  For now, this patch
> > prevents the oops.
> >
> 
> I have no idea what the semantics are.  All I'm saying is that it
> looks like the code still accesses memory past the end of the buffer.
> The buffer isn't a null pointer, so the symptom is different, but it
> may still be a security bug.

There's no accessing of data here, just writing the data out as an
extended attribute, which requires CAP_SYS_ADMIN privilege.

Mimi

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