Thanks for taking care of these items. I'll respond to your questions
here:

>> * get_crash_db_url(): needs to verify the url. Granted, it should be tightly 
>> controlled via the initscript or the root user, but still.
> Verified how? I could ensure there's an http:// on the front and a NUL 
> terminator on the end, but beyond that I'm unsure what else we could do.

Verified in a manner that will allow whoopsie to fail gracefully when
faced with invalid input. Checking for http:// or https:// and is nul
terminated is probably good enough, since this is administrator
controlled.

>>  * intially surprised that you disabled secure memory, but I guess
since this is a root process the contents of
/sys/class/dmi/id/product_uuid shouldn't leak out

> Is that really sensitive information to begin with? It's the
systemUUID. I can't envision an attack vector that involves it.

Well, this was more of a thought provoking comment than a firm
recommendation. The file is only accessible by root, but I couldn't
really think of an attack vector that would expose this via whoopsie
where the user was non-privileged that didn't require physical presence.
I might not have been creative enough, but it is easy enough to remove
later if needed.

>> * can DoS whoopsie by creating /tmp/.whoopsie-lock. Also symlink
race. Move to /var/lock/whoopsie instead (which has whoopsie write).
Maybe easier to just pass O_EXCL and check open() does not return -1.
This would not allow for the helpful error message though

> Is it really a symlink race if it doesn't write to the file?

Yes, you open it and can create an arbitrary file. It isn't a big deal
on 12.04 because we have Yama in the kernel to guard against it, but
still.

> The problem I see is that if it just uses open and O_CREAT | O_EXCL, if 
> something goes terribly wrong and whoopsie never removes /var/lock/whoopsie, 
> no further instances can be started.
>
>Made the change to use /var/lock/whoopsie in r152.

Hmm, well, /var/lock also has 1777 permissions, so this didn't address
the issue. What I meant was create the directory /var/local/whoopsie,
and put your lock file in there. Doing this is enough to fix both the
TOCTOU and the DoS.

> Given that /var/crash is world writable, why does this matter? Just
trying to understand what I'm defending against.

Because it is world writable, people can put anything they want in there
and whoopsie could process it. As such, whoopsie needs to be coded
defensively since it is processing untrusted and unfiltered content.
While is is excellent that whoospie is running as non-root at this
point, we still want to make sure that it handles invalid input
gracefully such that whoopsie won't crash or be subverted into executing
arbitrary code. I'm not sure I understand the TOCTOU you are referring
to here-- we just want parse_report() to fail gracefully in the presence
of invalid input. Once it is uploaded to the server, the server will of
course need to do its own input validation.

>> * polkit: 'gnome-control-center whoopsie-daisy'. verified to work correctly 
>> and verify permissions appropriately (ie, requires authentication for Set* 
>> operations, but not the Properties).
> The properties are marked as read-only, as defined in 
> whoopsie-preferences.xml. Attempting to set them through d-feet
returns: "org.freedesktop.DBus.Error.InvalidArgs: Property `ReportCrashes' is 
not writable."

I was not clear here. I was trying to say that the policykit parts were
done correctly. Sorry for the confusion.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/913694

Title:
  [MIR] whoopsie-daisy

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/whoopsie-daisy/+bug/913694/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to