The debian-private mailing list leak, part 1. Volunteers have complained about Blackmail. Lynchings. Character assassination. Defamation. Cyberbullying. Volunteers who gave many years of their lives are picked out at random for cruel social experiments. The former DPL's girlfriend Molly de Blanc is given volunteers to experiment on for her crazy talks. These volunteers never consented to be used like lab rats. We don't either. debian-private can no longer be a safe space for the cabal. Let these monsters have nowhere to hide. Volunteers are not disposable. We stand with the victims.
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
More xterm problems
I started to look for other potential security problems in xterm.
Simply grepping for strcpy turned up three situations where someone
might overwrite the stack by setting X resources. Whether they can be
exploited hinges on the maximum string length that xrdb allows. If
this is less than 1000, there should be no danger.
I only recently began learning about how to deal with security in
setuid programs. So please bear with me if I'm making mistakes.
The problems are in VTInitI18N() and HandleKeymapChange() in charproc.c.
I have included their code below, with line numbers, for reference.
I took the code from the XFree86 3.3 source tree on ftp.funet.fi.
In each case, a string supplied as a resource (thus, by the user) is
copied directly into an array allocated on the stack, with no length
checks. From what I've heard, this is the classic "buffer overrun".
Two of them are in VTInitI18N(), line numbers 3791 and 3826. Note
that this function is only compiled if XtSpecificationRelease >= 6, so
older systems are not affected. The resources used are inputMethod
and preeditType, both of the xterm widget. The first of these has
an additional complication, as the code performs some parsing on
the inputMethod string and assumes that the parse result will fit
in a 32-byte buffer. (line 3801).
The third is in HandleKeymapChange(), line 4374. This function is
registered as an action callback ("keymap") for the xterm widget, thus
the param[0] string it receives is also supplied by the user. This
string is copied without length checks, using sprintf, to an array
declared on the stack.
For both functions the fix is simple. In VTInitI18N(), slightly
rewrite the loops so that they do not have to modify the resource
string, and thus remove the need for a stack-allocated buffer to
hold a copy. The secondary problem at line 3801 is resolved by
truncating the parse result where necessary.
In HandleKeymapChange(), add a maximum string length to the sprintf
format string, making it impossible for the parameter to overflow the
buffer.
I do not foresee any compatibility problems with these truncations,
because the buffer sizes were already generous, and exceeding them was
already likely to cause a crash. If you want to be sure, bump the
buffer sizes a bit.
I have included a diff for these changes at the end of this mail.
This diff is intended for illustration only, since I have not tested
it except to check that the result compiles and runs normally. I am
not familiar enough with keymap support or the I18N extensions to test
it properly.
A spot check of the xrdb sources revealed no builtin limit on the
string length. I have not experimented because... well, because it's
11 am and I haven't slept yet.
If these are true security problems, I'll keep quiet about them until
you give me the ok. Feel free to forward this mail wherever
appropriate.
I hope this helps,
Richard Braakman
(now off to bed)
=== function charproc.c:VTInitI18N included for reference ===
3768 #if XtSpecificationRelease >= 6
3769 static void VTInitI18N()
3770 {
3771 int i;
3772 char *p,
3773 *s,
3774 *ns,
3775 *end,
3776 tmp[1024],
3777 buf[32];
3778 XIM xim = (XIM) NULL;
3779 XIMStyles *xim_styles;
3780 XIMStyle input_style = 0;
3781 Boolean found;
3782
3783 term->screen.xic = NULL;
3784
3785 if (!term->misc.open_im) return;
3786
3787 if (!term->misc.input_method || !*term->misc.input_method) {
3788 if ((p = XSetLocaleModifiers("@im=none")) != NULL && *p)
3789 xim = XOpenIM(XtDisplay(term), NULL, NULL, NULL);
3790 } else {
3791 strcpy(tmp, term->misc.input_method);
3792 for(ns=s=tmp; ns && *s;) {
3793 while (*s && isspace(*s)) s++;
3794 if (!*s) break;
3795 if ((ns = end = strchr(s, ',')) == 0)
3796 end = s + strlen(s);
3797 while (isspace(*end)) end--;
3798 *end = '\0';
3799
3800 strcpy(buf, "@im=");
3801 strcat(buf, s);
3802 if ((p = XSetLocaleModifiers(buf)) != NULL && *p
3803 && (xim = XOpenIM(XtDisplay(term), NULL, NULL, NULL)) != NULL)
3804 break;
3805
3806 s = ns + 1;
3807 }
3808 }
3809
3810 if (xim == NULL && (p = XSetLocaleModifiers("@im=none")) != NULL && *p)
3811 xim = XOpenIM(XtDisplay(term), NULL, NULL, NULL);
3812
3813 if (!xim) {
3814 fprintf(stderr, "Failed to open input method\n");
3815 return;
3816 }
3817
3818 if (XGetIMValues(xim, XNQueryInputStyle, &xim_styles, NULL)
3819 || !xim_styles) {
3820 fprintf(stderr, "input method doesn't support any style\n");
3821 XCloseIM(xim);
3822 return;
3823 }
3824
3825 found = False;
3826 strcpy(tmp, term->misc.preedit_type);
3827 for(s = tmp; s && !found;) {
3828 while (*s && isspace(*s)) s++;
3829 if (!*s) break;
3830 if ((ns = end = strchr(s, ',')) != 0)
3831 ns++;
3832 else
3833 end = s + strlen(s);
3834 while (isspace(*end)) end--;
3835 *end = '\0';
3836
3837 if (!strcmp(s, "OverTheSpot")) {
3838 input_style = (XIMPreeditPosition | XIMStatusArea);
3839 } else if (!strcmp(s, "OffTheSpot")) {
3840 input_style = (XIMPreeditArea | XIMStatusArea);
3841 } else if (!strcmp(s, "Root")) {
3842 input_style = (XIMPreeditNothing | XIMStatusNothing);
3843 }
3844 for (i = 0; (unsigned short)i < xim_styles->count_styles; i++)
3845 if (input_style == xim_styles->supported_styles[i]) {
3846 found = True;
3847 break;
3848 }
3849
3850 s = ns;
3851 }
3852 XFree(xim_styles);
3853
3854 if (!found) {
3855 fprintf(stderr, "input method doesn't support my preedit type\n");
3856 XCloseIM(xim);
3857 return;
3858 }
3859
3860 /*
3861 * This program only understands the Root preedit_style yet
3862 * Then misc.preedit_type should default to:
3863 * "OverTheSpot,OffTheSpot,Root"
3864 *
3865 * /MaF
3866 */
3867 if (input_style != (XIMPreeditNothing | XIMStatusNothing)) {
3868 fprintf(stderr,"This program only supports the 'Root' preedit type\n");
3869 XCloseIM(xim);
3870 return;
3871 }
3872
3873 term->screen.xic = XCreateIC(xim, XNInputStyle, input_style,
3874 XNClientWindow, term->core.window,
3875 XNFocusWindow, term->core.window,
3876 NULL);
3877
3878 if (!term->screen.xic) {
3879 fprintf(stderr,"Failed to create input context\n");
3880 XCloseIM(xim);
3881 }
3882
3883 return;
3884 }
3885 #endif
=== ===
=== function charproc.c:HandleKeymapChange included for reference ===
4351 /* ARGSUSED */
4352 static void HandleKeymapChange(w, event, params, param_count)
4353 Widget w;
4354 XEvent *event;
4355 String *params;
4356 Cardinal *param_count;
4357 {
4358 static XtTranslations keymap, original;
4359 static XtResource key_resources[] = {
4360 { XtNtranslations, XtCTranslations, XtRTranslationTable,
4361 sizeof(XtTranslations), 0, XtRTranslationTable, (XtPointer)NULL}
4362 };
4363 char mapName[1000];
4364 char mapClass[1000];
4365
4366 if (*param_count != 1) return;
4367
4368 if (original == NULL) original = w->core.tm.translations;
4369
4370 if (strcmp(params[0], "None") == 0) {
4371 XtOverrideTranslations(w, original);
4372 return;
4373 }
4374 (void) sprintf( mapName, "%sKeymap", params[0] );
4375 (void) strcpy( mapClass, mapName );
4376 if (islower(mapClass[0])) mapClass[0] = toupper(mapClass[0]);
4377 XtGetSubresources( w, (XtPointer)&keymap, mapName, mapClass,
4378 key_resources, (Cardinal)1, NULL, (Cardinal)0 );
4379 if (keymap != NULL)
4380 XtOverrideTranslations(w, keymap);
4381 }
=== ===
--- xterm-old/charproc.c Sun May 25 14:29:00 1997
+++ xterm-fix/charproc.c Mon Aug 11 10:43:32 1997
@@ -3773,7 +3773,6 @@
*s,
*ns,
*end,
- tmp[1024],
buf[32];
XIM xim = (XIM) NULL;
XIMStyles *xim_styles;
@@ -3788,17 +3787,18 @@
if ((p = XSetLocaleModifiers("@im=none")) != NULL && *p)
xim = XOpenIM(XtDisplay(term), NULL, NULL, NULL);
} else {
- strcpy(tmp, term->misc.input_method);
- for(ns=s=tmp; ns && *s;) {
+ for(ns=s=term->misc.input_method; ns && *s;) {
while (*s && isspace(*s)) s++;
if (!*s) break;
if ((ns = end = strchr(s, ',')) == 0)
end = s + strlen(s);
while (isspace(*end)) end--;
- *end = '\0';
strcpy(buf, "@im=");
- strcat(buf, s);
+ if (end - s >= sizeof(buf) - 4)
+ end = s + (sizeof(buf) - 5);
+ strncat(buf, s, end - s);
+
if ((p = XSetLocaleModifiers(buf)) != NULL && *p
&& (xim = XOpenIM(XtDisplay(term), NULL, NULL, NULL)) != NULL)
break;
@@ -3823,8 +3823,7 @@
}
found = False;
- strcpy(tmp, term->misc.preedit_type);
- for(s = tmp; s && !found;) {
+ for(s = term->misc.preedit_type; s && !found;) {
while (*s && isspace(*s)) s++;
if (!*s) break;
if ((ns = end = strchr(s, ',')) != 0)
@@ -3832,13 +3831,12 @@
else
end = s + strlen(s);
while (isspace(*end)) end--;
- *end = '\0';
- if (!strcmp(s, "OverTheSpot")) {
+ if (!strncmp(s, "OverTheSpot", end - s)) {
input_style = (XIMPreeditPosition | XIMStatusArea);
- } else if (!strcmp(s, "OffTheSpot")) {
+ } else if (!strncmp(s, "OffTheSpot", end - s)) {
input_style = (XIMPreeditArea | XIMStatusArea);
- } else if (!strcmp(s, "Root")) {
+ } else if (!strncmp(s, "Root", end - s)) {
input_style = (XIMPreeditNothing | XIMStatusNothing);
}
for (i = 0; (unsigned short)i < xim_styles->count_styles; i++)
@@ -4371,7 +4369,7 @@
XtOverrideTranslations(w, original);
return;
}
- (void) sprintf( mapName, "%sKeymap", params[0] );
+ (void) sprintf( mapName, "%900sKeymap", params[0] );
(void) strcpy( mapClass, mapName );
if (islower(mapClass[0])) mapClass[0] = toupper(mapClass[0]);
XtGetSubresources( w, (XtPointer)&keymap, mapName, mapClass,
--
TO UNSUBSCRIBE FROM THIS MAILING LIST: e-mail the word "unsubscribe" to
debian-private-request@lists.debian.org .
Trouble? e-mail to templin@bucknell.edu .