Bug 9160 - XQueryColors doesn't bounds-check its ncolors argument
XQueryColors doesn't bounds-check its ncolors argument
Status: RESOLVED FIXED
Product: xorg
Classification: Unclassified
Component: Lib/Xlib
git
All All
: high normal
Assigned To: Xorg Project Team
Xorg Project Team
2011BRB_Reviewed
: patch
Depends on:
Blocks: xorg-7.5
  Show dependency treegraph
 
Reported: 2006-11-25 19:30 UTC by Jamey Sharp
Modified: 2011-12-15 16:46 UTC (History)
2 users (show)

See Also:


Attachments
Kusanagi Kouichi's suggested fix (2.33 KB, patch)
2006-11-26 03:54 UTC, Jamey Sharp
no flags Details | Splinter Review
patch v2 (1.39 KB, patch)
2010-12-18 09:02 UTC, Kusanagi Kouichi
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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