Bug 9160

Summary: XQueryColors doesn't bounds-check its ncolors argument
Product: xorg Reporter: Jamey Sharp <jamey>
Component: Lib/XlibAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: high CC: josh, matthieu.herrb
Version: gitKeywords: patch
Hardware: All   
OS: All   
Whiteboard: 2011BRB_Reviewed
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 16399    
Attachments:
Description Flags
Kusanagi Kouichi's suggested fix
none
patch v2 none

Description Jamey Sharp 2006-11-25 19:30:52 UTC 
As reported in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=278984,
XQueryColors has poor behavior if more colors are passed than the server's core
maximum request length minus 2 (generally 65533 colors). Prior to the
introduction of XCB, the client would generally just hang, or sometimes report a
BadRequest error. With XCB, under some circumstances this bug triggers an
assertion failure in the client instead.

Xlib is delivering the right amount of data to the server, but overflowing the
16-bit request length field. It should split the request into chunks that fit in
the core maximum request length, and use an async reply handler to make all the
synchronous requests in one round-trip.
Comment 1 Jamey Sharp 2006-11-26 03:54:23 UTC 
Created attachment 7902 [details] [review]
Kusanagi Kouichi's suggested fix

In the Debian bug report for this issue, Kusanagi Kouichi
<slash@ma.neweb.ne.jp> provided this proposed patch; I missed it on first
reading of the mail. On quick inspection, it looks like the right fix, except
that ideally it would use an async reply handler to issue all the requests in
one round-trip. I'm also not sure how apps would deal with multiple X errors
from a single call to XQueryColors.
Comment 2 Daniel Stone 2007-02-27 01:34:49 UTC 
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 3 Daniel Stone 2009-08-31 18:15:07 UTC 
yeah, this should get fixed up for 7.5.
Comment 4 Matt Turner 2010-12-03 14:11:38 UTC 
Sent this to the mailing list. This bug report's not doing us any good, so I'll close it.
Comment 5 Julien Cristau 2010-12-04 04:20:23 UTC 
wtf.
Comment 6 Kusanagi Kouichi 2010-12-18 09:02:52 UTC 
Created attachment 41238 [details] [review]
patch v2

Use big request if the server supports it.
Comment 7 Jeremy Huddleston Sequoia 2011-10-07 15:28:42 UTC 
Could you please send this to xorg-devel for review, so we can close this up?
Comment 8 Alan Coopersmith 2011-12-15 16:46:38 UTC 
Has now been pushed to git master:
http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=87e10a7b9a97c951ab4d477f61177779ac0a6a66

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.