| Summary: | Buffer overflow in WriteCHdrKeyTypes running 'xkbcomp -C :0' | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Bryce Harrington <bryce> | ||||||||||||
| Component: | App/xkbcomp | Assignee: | Peter Hutterer <peter.hutterer> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> | ||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | medium | CC: | kees | ||||||||||||
| Version: | 7.3 (2007.09) | ||||||||||||||
| Hardware: | Other | ||||||||||||||
| OS: | All | ||||||||||||||
| URL: | https://bugs.edge.launchpad.net/ubuntu/+source/x11-xkb-utils/+bug/309013 | ||||||||||||||
| Whiteboard: | |||||||||||||||
| i915 platform: | i915 features: | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Bryce Harrington
2009-01-09 17:04:36 UTC
Created attachment 21848 [details] [review] handles other crashing strcpy's patch looks good in principle, minus the problem that asprintf isn't available on at least Solaris. Can you re-do the patch avoiding the use of asprintf please? (and while you're at it, a git-formatted patch would be nice :) I used asprintf because I have no idea what the maximum length of the returned string from XkbAtomText is. I assume you could fix this issue using a corrected maximum size to the local stack buffer. How long can the result string be expected to be? > I used asprintf because I have no idea what the maximum length of the returned
> string from XkbAtomText is. I assume you could fix this issue using a
> corrected maximum size to the local stack buffer. How long can the result
> string be expected to be?
shouldn't something like
--
char* foo = XkbAtomText(..);
char* bar = malloc(strlen(foo));
strcpy(bar, foo);
--
do the same job as asprintf?
Created attachment 21912 [details] [review] use strdup Argh. Sorry, I've been debugging sprintf issues in other code for so long that I forgot about strdup. Duh. This should be portable. getting there. instead of perror it's probably better to use _XkbLibError. and exit(1) in a library is a big no-no. Just return 0. (yes, I know, I should have caught that in the first review) Created attachment 21913 [details] [review] use XkbLibError, don't exit How about this? Looks like WriteTypeInitFunc is void, so there's nothing to do but return after the error report. > How about this? Looks like WriteTypeInitFunc is void, so there's nothing to do
> but return after the error report.
I think it' be better to just break out of the loop, since otherwise we leave
a dangling { in the output. The same with the rest of it, leaving unclosed {
around is probably bad. (I'm not sure how much that'll affect it though since
the output will be mostly empty anyway.)
Other than that, fine with me. Please attach a git-formatted patch so I can
push it easily.
Created attachment 21956 [details] [review] break from loop after writing out an #error In an effort to put errors somewhere when the alloc fails, we can just write out an error to the C file, and continue the loop. Attach as a git patch. Pushed as dd9514fe714d81b881a1bd6bd88d4287adc5fc7e. Thanks. (Now, if you have the time to figure out why the geometry still has BadAllocs in some cases, that'd be much appreciated :) |
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.