Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review locking/multi-threading implementation #36

Open
michaelrsweet opened this issue Sep 26, 2024 · 5 comments
Open

Review locking/multi-threading implementation #36

michaelrsweet opened this issue Sep 26, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@michaelrsweet
Copy link
Member

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.

@michaelrsweet michaelrsweet added the bug Something isn't working label Sep 26, 2024
@evilsocket
Copy link

evilsocket commented Sep 26, 2024

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
Copy link
Member Author

@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
This comment was marked as a violation of GitHub Acceptable Use Policies
@evilsocket
Copy link

And YES, macOS IS affected, wait for it.

@OpenPrinting OpenPrinting locked as too heated and limited conversation to collaborators Sep 26, 2024
@michaelrsweet
Copy link
Member Author

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants