971370 - Errors in seccomp whitelist due to bug in strace (msgget, recv, getdents64)

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I'm not sure why we're whitelisting msgget.  The only code in Gecko that currently uses it is sipcc in webrtc, which doesn't work in any case because of msgsnd/msgrcv (and maybe msgctl?), and there are plans to replace that code (see bug 966547).  It seems to be possible to remove it without breaking things — and I can't imagine how it could be useful without the rest of the msg* family — but that should probably be tested a little more.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Yes, we can.  It's a bug in strace.  I "enhanced" it to display the syscall numbers along with the names; here are two representative lines:

msgget=263(0x1, 0xbebcfee8, 0, 0x1)     = 0
semget=252(0x15, 0xbebcfdc8, 0x10, 0)   = 0

Here are the actual syscalls:

#define __NR_clock_gettime (__NR_SYSCALL_BASE+263)
#define __NR_epoll_wait (__NR_SYSCALL_BASE+252)

This would also explain why the mysterious alleged msgget calls continued after de-whitelisting msgget, and why semget (which has never been whitelisted, I think?) appears.

(This might also explain some of the "ptrace: umoven: ..." errors I've seen, if strace's syscall confusion is such that it sometimes treats an argument as a string or otherwise passed in memory when it's not actually a pointer.)

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

It gets worse.

recv=240(1076905460, ptrace: umoven: I/O error
getdents64=220(0x471e4000, 0x2000, 0x4, 0xb36) = 0

#define __NR_futex (__NR_SYSCALL_BASE+240)
#define __NR_madvise (__NR_SYSCALL_BASE+220)

The getdents64 thing makes sense if it's using the i386 syscall table on ARM (!!!!!), but there's something else going on with everything ipc-related -- I'd guess it's something to do with Linux/i386's multiplexed "ipc" syscall (because i386 syscall 240 is also futex).

I *think* that's all of it.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

But we actually do use getdents64.

 0  libc.so + 0xc808
 1  libc.so!readdir [opendir.c : 145 + 0x5]
 2  libxul.so!nsDirEnumeratorUnix::GetNextEntry() [nsLocalFileUnix.cpp : 176 + 0x5]
 3  libxul.so!nsDirEnumeratorUnix::Init(nsLocalFile*, bool) [nsLocalFileUnix.cpp : 148 + 0x7]
 4  libxul.so!nsLocalFile::GetDirectoryEntries(nsISimpleEnumerator**) [nsLocalFileUnix.cpp : 1722 + 0xb]
 5  libxul.so!mozHunspell::LoadDictionariesFromDir(nsIFile*) [mozHunspell.cpp : 470 + 0x3]
 6  libxul.so!mozHunspell::LoadDictionaryList() [mozHunspell.cpp : 400 + 0x9]
 7  libxul.so!mozHunspell::Init() [mozHunspell.cpp : 124 + 0x3]
 8  libxul.so!mozHunspellConstructor [mozSpellCheckerFactory.cpp : 27 + 0xd]

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug971370-strace-considered-harmful-hg0.diff

Trying: https://tbpl.mozilla.org/?tree=Try&rev=1eaa6ae53859

Comment 5

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8375098 [details] [diff] [review]
bug971370-strace-considered-harmful-hg0.diff

Review of attachment 8375098 [details] [diff] [review]:
-----------------------------------------------------------------

yep that strace got me .. :(

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Try is no more broken than without this patch: https://tbpl.mozilla.org/?tree=Try&rev=1eaa6ae53859

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/935cb4d15364

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/935cb4d15364