OpenPrinting / cups-browsed

Apache License 2.0
37 stars 10 forks source link

Review locking/multi-threading implementation #36

Open michaelrsweet opened 1 month ago

michaelrsweet commented 1 month ago

According to @evilsocket, cups-browsed can be held up for an extended period of time:

The lock acquired here doesn't get unlocked until the IPP server has responded. A malicious IPP server can keep the connection going effectively remotely causing a DoS of the service.

I believe there's also a race condition because internally examine_discovered_printer_record then tries to unlock and relocks after a while.

We should review the locking and multi-threading implementation in cups-browsed to ensure we don't deadlock and don't wait indefinitely for a printer to respond. In particular, a short timeout (10 seconds?) should be set on any printer connection so that we minimize the chances of a misbehaving printer from preventing cups-browsed from setting up local print queues for legacy applications.

evilsocket commented 1 month ago

Do you realize that there's tons of people observing these repos and the commits you have been pushing on the public branches for days now instead of following the suggested procedure for GHSAs?

michaelrsweet commented 1 month ago

@evilsocket I have only pushed changes to the private branch of this project (cups-browsed) and to the private branches of libcupsfilters and libppd for the issue you reported. The CUPS repository is a completely separate project (which is why macOS isn't affected...) and NOT subject to the GHSAs you filed.

WRT this separate issue, cups-browsed is only used to create local queues for legacy applications that aren't using the "modern" (released in CUPS 1.1 - July 2000) CUPS printing APIs. If we have a DoS here, some users won't be able to print but that's it.

evilsocket commented 1 month ago

@michaelrsweet are you for fucking real man? Do you think people are stupid?

https://github.com/OpenPrinting/cups/commit/96b3bdf010e78880f5764e5032720379aa1116df

evilsocket commented 1 month ago

And YES, macOS IS affected, wait for it.

michaelrsweet commented 1 month ago

@evilsocket Discussion is now locked.

macOS doesn't ship the extra OpenPrinting code and runs CUPS and its filters in a sandbox with minimal privileges which effectively prevents the OpenPrinting attacks.

The changes to the CUPS repository are for hardening the manual printer creation process where a user has requested that cupsd create a print queue. There may be similarities in the changes but the focus is on closing potential attack vectors in related (but as of yet) unaffected software.