Xrm bug is there, too, and I have made my version of its patch --
one-line fix by David Hedley works with his test, but xc/lib/X11/Xrm.c has
other places where pointers can step over buffers boundaries (some parts
of code that are related to value parsing have boundaries checking, others
don't). Also some library functions make rather unobvious assumptions
about buffers that user supplied to them, but in themselves they look
safe.
Patches:
For environment buffer overflows and NULL display:
---8<---
diff -u old-xc/lib/X11/lcFile.c xc/lib/X11/lcFile.c
--- old-xc/lib/X11/lcFile.c Sat Sep 28 13:46:16 1996
+++ xc/lib/X11/lcFile.c Wed May 28 09:13:36 1997
@@ -267,10 +267,14 @@
xlocaledir(dir,BUFSIZE);
n = _XlcParsePath(dir, args, 256);
for(i = 0; i < n; ++i){
- if ((2 + (args[i] ? strlen (args[i]) : 0) +
- strlen (locale_alias)) < BUFSIZE) {
- sprintf(buf, "%s/%s", args[i], locale_alias);
- name = _XlcResolveName(lc_name, buf, LtoR);
+ if (args[i]){
+ if ((2 + strlen (args[i]) +
+ strlen (locale_alias)) < BUFSIZE) {
+ sprintf(buf, "%s/%s", args[i], locale_alias);
+ name = _XlcResolveName(lc_name, buf, LtoR);
+ } else {
+ name = NULL;
+ }
}
if(name != NULL){
break;
@@ -278,12 +282,25 @@
}
if(name != NULL){
- strcpy(buf, name);
+ if(strlen(name) < BUFSIZE - 1){
+ strcpy(buf, name);
+ } else {
+ fprintf(stderr,
+"Warning: locale \"%s\" is too long, ignored\n", name);
+ *buf = '\0';
+ }
Xfree(name);
}else{
- strcpy(buf, lc_name);
+ if(strlen(lc_name) < BUFSIZE - 1){
+ strcpy(buf, lc_name);
+ } else {
+ fprintf(stderr,
+"Warning: locale \"%s\" is too long, ignored\n", lc_name);
+ *buf = '\0';
+ }
}
if(full_name != NULL){
+/* This argument always has BUFSIZE bytes allocated if not NULL */
strcpy(full_name, buf);
}
@@ -296,25 +313,49 @@
if(territory) *territory = '\0';
if(codeset) *codeset = '\0';
- name_p = buf;
- ptr = language;
- while (1) {
- if (*name_p == '_') {
- if (ptr)
- *ptr = '\0';
- ptr = territory;
- } else if (*name_p == '.') {
- if (ptr)
- *ptr = '\0';
- ptr = codeset;
- } else {
- if (ptr)
- *ptr++ = *name_p;
- if (*name_p == '\0')
- break;
- }
- name_p++;
- }
+ name_p = buf;
+ ptr = strchr(name_p, '_');
+ if(!ptr) ptr = strchr(name_p, '.');
+ if(!ptr) ptr = name_p + strlen(name_p);
+/* 128 because it's the size of that buffer */
+ if(ptr - name_p < 128) {
+ if(language) {
+ memcpy(language,name_p, ptr - name_p);
+ language[ptr - name_p]=0;
+ }
+ }else{
+ fprintf(stderr,
+"Warning: language name in locale \"%s\" is too long, ignored\n", buf);
+ }
+ if(*ptr == '_') {
+ name_p = ptr + 1;
+ ptr = strchr(name_p, '.');
+ if(!ptr) ptr = name_p + strlen(name_p);
+/* 128 because it's the size of that buffer */
+ if(ptr - name_p < 128) {
+ if(territory) {
+ memcpy(territory, name_p, ptr - name_p);
+ territory[ptr - name_p]=0;
+ }
+ }else{
+ fprintf(stderr,
+"Warning: territory name in locale \"%s\" is too long, ignored\n", buf);
+ }
+ }
+ if(*ptr == '.') {
+ name_p = ptr+1;
+ ptr = name_p + strlen(name_p);
+/* 128 because it's the size of that buffer */
+ if(ptr - name_p < 128) {
+ if(codeset) {
+ memcpy(codeset, name_p, ptr-name_p);
+ codeset[ptr - name_p]=0;
+ }
+ }else{
+ fprintf(stderr,
+"Warning: codeset name in locale \"%s\" is too long, ignored\n", buf);
+ }
+ }
}
return (buf[0] != '\0') ? 1 : 0;
diff -u old-xc/lib/Xt/Initialize.c xc/lib/Xt/Initialize.c
--- old-xc/lib/Xt/Initialize.c Wed Dec 4 07:25:29 1996
+++ xc/lib/Xt/Initialize.c Wed May 28 07:24:15 1997
@@ -938,7 +938,8 @@
argc_in_out, &argv_in_out, fallback_resources);
LOCK_APP(app_con);
- XtSetArg(args[num], XtNscreen, DefaultScreenOfDisplay(dpy)); num++;
+ if(dpy)
+ XtSetArg(args[num], XtNscreen, DefaultScreenOfDisplay(dpy)); num++;
XtSetArg(args[num], XtNargc, saved_argc); num++;
XtSetArg(args[num], XtNargv, argv_in_out); num++;
diff -u old-xc/lib/Xt/Intrinsic.c xc/lib/Xt/Intrinsic.c
--- old-xc/lib/Xt/Intrinsic.c Sat Sep 28 13:54:39 1996
+++ xc/lib/Xt/Intrinsic.c Tue May 27 04:44:01 1997
@@ -1109,6 +1109,7 @@
char *start;
char *end;
int len;
+ int buflen;
#ifdef SKIPCOUNT
int n;
#endif
@@ -1129,8 +1130,10 @@
#endif
if (end = strchr (start, ENDCHAR)) {
len = end - start;
- strncpy(buf, start, len);
- *(buf + len) = '\0';
+ buflen=len;
+ if(buflen>=MAXLOCALE) buflen=MAXLOCALE-1;
+ strncpy(buf, start, buflen+1);
+ *(buf + buflen) = '\0';
#ifdef WHITEFILL
for (start = buf; start = strchr(start, ' '); )
*start++ = '-';
--->8---
For Xrm buffer overflows:
---8<---
--- old-xc/lib/X11/Xrm.c Thu Jun 8 20:20:39 1995
+++ xc/lib/X11/Xrm.c Thu May 29 14:07:49 1997
@@ -1179,10 +1179,11 @@
* storing characters and converting this to a Quark.
*
* If the number of quarks is greater than LIST_SIZE - 1. This
- * function will trash your memory.
+ * function was able to trash your memory, so now checks for
+ * boundaries added.
*
* If the length of any quark is larger than BUFSIZ this function
- * will also trash memory.
+ * was also able trash memory, so boundaries checks are added, too.
*/
t_bindings = bindings;
@@ -1191,26 +1192,29 @@
sig = 0;
ptr = buffer;
*t_bindings = XrmBindTightly;
- for(;;) {
+ while(ptr - buffer < BUFSIZ && t_quarks - quarks < LIST_SIZE - 1) {
+
if (!is_binding(bits)) {
- while (!is_EOQ(bits)) {
+ while (!is_EOQ(bits) && (ptr - buffer < BUFSIZ)) {
*ptr++ = c;
sig = (sig << 1) + c; /* Compute the signature. */
bits = next_char(c, str);
}
-
- *t_quarks++ = _XrmInternalStringToQuark(buffer, ptr - buffer,
- sig, False);
+ if(t_quarks-quarks < LIST_SIZE - 1)
+ *t_quarks++ = _XrmInternalStringToQuark(buffer, ptr - buffer,
+ sig, False);
if (is_separator(bits)) {
if (!is_space(bits))
break;
/* Remove white space */
- do {
- *ptr++ = c;
- sig = (sig << 1) + c; /* Compute the signature. */
- } while (is_space(bits = next_char(c, str)));
+ if(ptr - buffer < BUFSIZ) {
+ do {
+ *ptr++ = c;
+ sig = (sig << 1) + c; /* Compute the signature. */
+ } while (is_space(bits = next_char(c, str)) && (ptr - buffer < BUFSIZ));
+ }
/*
* The spec doesn't permit it, but support spaces
@@ -1223,10 +1227,12 @@
continue;
}
- if (c == '.')
- *(++t_bindings) = XrmBindTightly;
- else
- *(++t_bindings) = XrmBindLoosely;
+ if(t_bindings - bindings < LIST_SIZE - 1) {
+ if (c == '.')
+ *(++t_bindings) = XrmBindTightly;
+ else
+ *(++t_bindings) = XrmBindLoosely;
+ }
sig = 0;
ptr = buffer;
--->8---
-- Alex