Package: emacs;
Reported by: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Mon, 23 May 2011 06:54:01 UTC
Severity: normal
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8719 in the body.
You can then email your comments to 8719 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:bug#8719; Package emacs.
(Mon, 23 May 2011 06:54:02 GMT) Full text and rfc822 format available.Paul Eggert <eggert <at> cs.ucla.edu>:bug-gnu-emacs <at> gnu.org.
(Mon, 23 May 2011 06:54:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Paul Eggert <eggert <at> cs.ucla.edu> To: bug-gnu-emacs <at> gnu.org Subject: ccl: add some integer overflow checks Date: Sun, 22 May 2011 23:53:27 -0700
I did a quick pass through ccl.c and found several places where
integer overflow could cause problems. Here's a proposed fix for some
of the problems.
Does anybody have a good suggestion for testing these? I'm no
expert in CCL.
ccl: add integer overflow checks
* ccl.c (CCL_CODE_MAX, GET_CCL_RANGE, GET_CCL_CODE, GET_CCL_INT):
(IN_INT_RANGE): New macros.
(ccl_driver): Use them to check for integer overflow when
decoding a CCL program. Many of the new checks are whether XINT (x)
fits in int; it doesn't always, on 64-bit hosts. The new version
doesn't catch all possible integer overflows, but it's an
improvement.
=== modified file 'src/ccl.c'
--- src/ccl.c 2011-05-12 07:07:06 +0000
+++ src/ccl.c 2011-05-23 06:46:58 +0000
@@ -98,6 +98,8 @@
and `rrr' are CCL register number, `XXXXX' is one of the following
CCL commands. */
+#define CCL_CODE_MAX ((1 << (28 - 1)) - 1)
+
/* CCL commands
Each comment fields shows one or more lines for command syntax and
@@ -742,6 +744,24 @@
#endif
+#define GET_CCL_RANGE(var, ccl_prog, ic, lo, hi) \
+ do \
+ { \
+ EMACS_INT prog_word = XINT ((ccl_prog)[ic]); \
+ if (! ((lo) <= prog_word && prog_word <= (hi))) \
+ CCL_INVALID_CMD; \
+ (var) = prog_word; \
+ } \
+ while (0)
+
+#define GET_CCL_CODE(code, ccl_prog, ic) \
+ GET_CCL_RANGE (code, ccl_prog, ic, 0, CCL_CODE_MAX)
+
+#define GET_CCL_INT(var, ccl_prog, ic) \
+ GET_CCL_RANGE (var, ccl_prog, ic, INT_MIN, INT_MAX)
+
+#define IN_INT_RANGE(val) (INT_MIN <= (val) && (val) <= INT_MAX)
+
/* Encode one character CH to multibyte form and write to the current
output buffer. If CH is less than 256, CH is written as is. */
#define CCL_WRITE_CHAR(ch) \
@@ -899,7 +919,7 @@
}
this_ic = ic;
- code = XINT (ccl_prog[ic]); ic++;
+ GET_CCL_CODE (code, ccl_prog, ic++);
field1 = code >> 8;
field2 = (code & 0xFF) >> 5;
@@ -920,15 +940,14 @@
break;
case CCL_SetConst: /* 00000000000000000000rrrXXXXX */
- reg[rrr] = XINT (ccl_prog[ic]);
- ic++;
+ GET_CCL_INT (reg[rrr], ccl_prog, ic++);
break;
case CCL_SetArray: /* CCCCCCCCCCCCCCCCCCCCRRRrrrXXXXX */
i = reg[RRR];
j = field1 >> 3;
if ((unsigned int) i < j)
- reg[rrr] = XINT (ccl_prog[ic + i]);
+ GET_CCL_INT (reg[rrr], ccl_prog, ic + i);
ic += j;
break;
@@ -956,13 +975,13 @@
break;
case CCL_WriteConstJump: /* A--D--D--R--E--S--S-000XXXXX */
- i = XINT (ccl_prog[ic]);
+ GET_CCL_INT (i, ccl_prog, ic);
CCL_WRITE_CHAR (i);
ic += ADDR;
break;
case CCL_WriteConstReadJump: /* A--D--D--R--E--S--S-rrrXXXXX */
- i = XINT (ccl_prog[ic]);
+ GET_CCL_INT (i, ccl_prog, ic);
CCL_WRITE_CHAR (i);
ic++;
CCL_READ_CHAR (reg[rrr]);
@@ -970,18 +989,17 @@
break;
case CCL_WriteStringJump: /* A--D--D--R--E--S--S-000XXXXX */
- j = XINT (ccl_prog[ic]);
- ic++;
+ GET_CCL_INT (j, ccl_prog, ic++);
CCL_WRITE_STRING (j);
ic += ADDR - 1;
break;
case CCL_WriteArrayReadJump: /* A--D--D--R--E--S--S-rrrXXXXX */
i = reg[rrr];
- j = XINT (ccl_prog[ic]);
+ GET_CCL_INT (j, ccl_prog, ic);
if ((unsigned int) i < j)
{
- i = XINT (ccl_prog[ic + 1 + i]);
+ GET_CCL_INT (i, ccl_prog, ic + 1 + i);
CCL_WRITE_CHAR (i);
}
ic += j + 2;
@@ -998,10 +1016,14 @@
CCL_READ_CHAR (reg[rrr]);
/* fall through ... */
case CCL_Branch: /* CCCCCCCCCCCCCCCCCCCCrrrXXXXX */
- if ((unsigned int) reg[rrr] < field1)
- ic += XINT (ccl_prog[ic + reg[rrr]]);
- else
- ic += XINT (ccl_prog[ic + field1]);
+ {
+ int incr;
+ GET_CCL_INT (incr, ccl_prog,
+ ic + ((unsigned int) reg[rrr] < field1
+ ? reg[rrr]
+ : field1));
+ ic += incr;
+ }
break;
case CCL_ReadRegister: /* CCCCCCCCCCCCCCCCCCCCrrXXXXX */
@@ -1009,7 +1031,7 @@
{
CCL_READ_CHAR (reg[rrr]);
if (!field1) break;
- code = XINT (ccl_prog[ic]); ic++;
+ GET_CCL_CODE (code, ccl_prog, ic++);
field1 = code >> 8;
field2 = (code & 0xFF) >> 5;
}
@@ -1018,7 +1040,7 @@
case CCL_WriteExprConst: /* 1:00000OPERATION000RRR000XXXXX */
rrr = 7;
i = reg[RRR];
- j = XINT (ccl_prog[ic]);
+ GET_CCL_INT (j, ccl_prog, ic);
op = field1 >> 6;
jump_address = ic + 1;
goto ccl_set_expr;
@@ -1029,7 +1051,7 @@
i = reg[rrr];
CCL_WRITE_CHAR (i);
if (!field1) break;
- code = XINT (ccl_prog[ic]); ic++;
+ GET_CCL_CODE (code, ccl_prog, ic++);
field1 = code >> 8;
field2 = (code & 0xFF) >> 5;
}
@@ -1051,10 +1073,7 @@
/* If FFF is nonzero, the CCL program ID is in the
following code. */
if (rrr)
- {
- prog_id = XINT (ccl_prog[ic]);
- ic++;
- }
+ GET_CCL_INT (prog_id, ccl_prog, ic++);
else
prog_id = field1;
@@ -1097,7 +1116,7 @@
i = reg[rrr];
if ((unsigned int) i < field1)
{
- j = XINT (ccl_prog[ic + i]);
+ GET_CCL_INT (j, ccl_prog, ic + i);
CCL_WRITE_CHAR (j);
}
ic += field1;
@@ -1122,8 +1141,7 @@
CCL_SUCCESS;
case CCL_ExprSelfConst: /* 00000OPERATION000000rrrXXXXX */
- i = XINT (ccl_prog[ic]);
- ic++;
+ GET_CCL_INT (i, ccl_prog, ic++);
op = field1 >> 6;
goto ccl_expr_self;
@@ -1159,9 +1177,9 @@
case CCL_SetExprConst: /* 00000OPERATION000RRRrrrXXXXX */
i = reg[RRR];
- j = XINT (ccl_prog[ic]);
+ GET_CCL_INT (j, ccl_prog, ic++);
op = field1 >> 6;
- jump_address = ++ic;
+ jump_address = ic;
goto ccl_set_expr;
case CCL_SetExprReg: /* 00000OPERATIONRrrRRRrrrXXXXX */
@@ -1175,10 +1193,9 @@
CCL_READ_CHAR (reg[rrr]);
case CCL_JumpCondExprConst: /* A--D--D--R--E--S--S-rrrXXXXX */
i = reg[rrr];
- op = XINT (ccl_prog[ic]);
- jump_address = ic++ + ADDR;
- j = XINT (ccl_prog[ic]);
- ic++;
+ jump_address = ic + ADDR;
+ GET_CCL_INT (op, ccl_prog, ic++);
+ GET_CCL_INT (j, ccl_prog, ic++);
rrr = 7;
goto ccl_set_expr;
@@ -1186,10 +1203,10 @@
CCL_READ_CHAR (reg[rrr]);
case CCL_JumpCondExprReg:
i = reg[rrr];
- op = XINT (ccl_prog[ic]);
- jump_address = ic++ + ADDR;
- j = reg[XINT (ccl_prog[ic])];
- ic++;
+ jump_address = ic + ADDR;
+ GET_CCL_INT (op, ccl_prog, ic++);
+ GET_CCL_RANGE (j, ccl_prog, ic++, 0, 7);
+ j = reg[j];
rrr = 7;
ccl_set_expr:
@@ -1267,18 +1284,27 @@
break;
case CCL_TranslateCharacterConstTbl:
- op = XINT (ccl_prog[ic]); /* table */
- ic++;
- i = CCL_DECODE_CHAR (reg[RRR], reg[rrr]);
- op = translate_char (GET_TRANSLATION_TABLE (op), i);
- CCL_ENCODE_CHAR (op, charset_list, reg[RRR], reg[rrr]);
+ {
+ EMACS_INT eop;
+ GET_CCL_RANGE (eop, ccl_prog, ic++, 0,
+ (VECTORP (Vtranslation_table_vector)
+ ? ASIZE (Vtranslation_table_vector)
+ : -1));
+ i = CCL_DECODE_CHAR (reg[RRR], reg[rrr]);
+ op = translate_char (GET_TRANSLATION_TABLE (eop), i);
+ CCL_ENCODE_CHAR (op, charset_list, reg[RRR], reg[rrr]);
+ }
break;
case CCL_LookupIntConstTbl:
- op = XINT (ccl_prog[ic]); /* table */
- ic++;
{
- struct Lisp_Hash_Table *h = GET_HASH_TABLE (op);
+ EMACS_INT eop;
+ struct Lisp_Hash_Table *h;
+ GET_CCL_RANGE (eop, ccl_prog, ic++, 0,
+ (VECTORP (Vtranslation_hash_table_vector)
+ ? ASIZE (Vtranslation_hash_table_vector)
+ : -1));
+ h = GET_HASH_TABLE (eop);
op = hash_lookup (h, make_number (reg[RRR]), NULL);
if (op >= 0)
@@ -1297,18 +1323,22 @@
break;
case CCL_LookupCharConstTbl:
- op = XINT (ccl_prog[ic]); /* table */
- ic++;
- i = CCL_DECODE_CHAR (reg[RRR], reg[rrr]);
{
- struct Lisp_Hash_Table *h = GET_HASH_TABLE (op);
+ EMACS_INT eop;
+ struct Lisp_Hash_Table *h;
+ GET_CCL_RANGE (eop, ccl_prog, ic++, 0,
+ (VECTORP (Vtranslation_hash_table_vector)
+ ? ASIZE (Vtranslation_hash_table_vector)
+ : -1));
+ i = CCL_DECODE_CHAR (reg[RRR], reg[rrr]);
+ h = GET_HASH_TABLE (eop);
op = hash_lookup (h, make_number (i), NULL);
if (op >= 0)
{
Lisp_Object opl;
opl = HASH_VALUE (h, op);
- if (!INTEGERP (opl))
+ if (! (INTEGERP (opl) && IN_INT_RANGE (XINT (opl))))
CCL_INVALID_CMD;
reg[RRR] = XINT (opl);
reg[7] = 1; /* r7 true for success */
@@ -1321,9 +1351,10 @@
case CCL_IterateMultipleMap:
{
Lisp_Object map, content, attrib, value;
- int point, size, fin_ic;
+ EMACS_INT point, size;
+ int fin_ic;
- j = XINT (ccl_prog[ic++]); /* number of maps. */
+ GET_CCL_INT (j, ccl_prog, ic++); /* number of maps. */
fin_ic = ic + j;
op = reg[rrr];
if ((j > reg[RRR]) && (j >= 0))
@@ -1343,7 +1374,7 @@
size = ASIZE (Vcode_conversion_map_vector);
point = XINT (ccl_prog[ic++]);
- if (point >= size) continue;
+ if (! (0 <= point && point < size)) continue;
map = AREF (Vcode_conversion_map_vector, point);
/* Check map validity. */
@@ -1358,18 +1389,19 @@
/* check map type,
[STARTPOINT VAL1 VAL2 ...] or
[t ELEMENT STARTPOINT ENDPOINT] */
- if (NUMBERP (content))
+ if (INTEGERP (content))
{
- point = XUINT (content);
- point = op - point + 1;
- if (!((point >= 1) && (point < size))) continue;
- content = AREF (map, point);
+ point = XINT (content);
+ if (!(point <= op && op - point + 1 < size)) continue;
+ content = AREF (map, op - point + 1);
}
else if (EQ (content, Qt))
{
if (size != 4) continue;
- if ((op >= XUINT (AREF (map, 2)))
- && (op < XUINT (AREF (map, 3))))
+ if (INTEGERP (AREF (map, 2))
+ && XINT (AREF (map, 2)) <= op
+ && INTEGERP (AREF (map, 3))
+ && op < XINT (AREF (map, 3)))
content = AREF (map, 1);
else
continue;
@@ -1379,7 +1411,7 @@
if (NILP (content))
continue;
- else if (NUMBERP (content))
+ else if (INTEGERP (content) && IN_INT_RANGE (XINT (content)))
{
reg[RRR] = i;
reg[rrr] = XINT(content);
@@ -1394,10 +1426,11 @@
{
attrib = XCAR (content);
value = XCDR (content);
- if (!NUMBERP (attrib) || !NUMBERP (value))
+ if (! (INTEGERP (attrib) && INTEGERP (value)
+ && IN_INT_RANGE (XINT (value))))
continue;
reg[RRR] = i;
- reg[rrr] = XUINT (value);
+ reg[rrr] = XINT (value);
break;
}
else if (SYMBOLP (content))
@@ -1432,8 +1465,9 @@
mapping_stack_pointer = mapping_stack;
stack_idx_of_map_multiple = 0;
- map_set_rest_length =
- XINT (ccl_prog[ic++]); /* number of maps and separators. */
+ /* Get number of maps and separators. */
+ GET_CCL_INT (map_set_rest_length, ccl_prog, ic++);
+
fin_ic = ic + map_set_rest_length;
op = reg[rrr];
@@ -1501,7 +1535,7 @@
do {
for (;map_set_rest_length > 0;i++, ic++, map_set_rest_length--)
{
- point = XINT(ccl_prog[ic]);
+ GET_CCL_INT (point, ccl_prog, ic);
if (point < 0)
{
/* +1 is for including separator. */
@@ -1531,18 +1565,19 @@
/* check map type,
[STARTPOINT VAL1 VAL2 ...] or
[t ELEMENT STARTPOINT ENDPOINT] */
- if (NUMBERP (content))
+ if (INTEGERP (content))
{
- point = XUINT (content);
- point = op - point + 1;
- if (!((point >= 1) && (point < size))) continue;
- content = AREF (map, point);
+ point = XINT (content);
+ if (!(point <= op && op - point + 1 < size)) continue;
+ content = AREF (map, op - point + 1);
}
else if (EQ (content, Qt))
{
if (size != 4) continue;
- if ((op >= XUINT (AREF (map, 2))) &&
- (op < XUINT (AREF (map, 3))))
+ if (INTEGERP (AREF (map, 2))
+ && XINT (AREF (map, 2)) <= op
+ && INTEGERP (AREF (map, 3))
+ && op < XINT (AREF (map, 3)))
content = AREF (map, 1);
else
continue;
@@ -1554,7 +1589,7 @@
continue;
reg[RRR] = i;
- if (NUMBERP (content))
+ if (INTEGERP (content) && IN_INT_RANGE (XINT (content)))
{
op = XINT (content);
i += map_set_rest_length - 1;
@@ -1566,9 +1601,10 @@
{
attrib = XCAR (content);
value = XCDR (content);
- if (!NUMBERP (attrib) || !NUMBERP (value))
+ if (! (INTEGERP (attrib) && INTEGERP (value)
+ && IN_INT_RANGE (XINT (value))))
continue;
- op = XUINT (value);
+ op = XINT (value);
i += map_set_rest_length - 1;
ic += map_set_rest_length - 1;
POP_MAPPING_STACK (map_set_rest_length, reg[rrr]);
@@ -1613,7 +1649,7 @@
case CCL_MapSingle:
{
Lisp_Object map, attrib, value, content;
- int size, point;
+ int point;
j = XINT (ccl_prog[ic++]); /* map_id */
op = reg[rrr];
if (j >= ASIZE (Vcode_conversion_map_vector))
@@ -1628,41 +1664,36 @@
break;
}
map = XCDR (map);
- if (!VECTORP (map))
+ if (! (VECTORP (map)
+ && INTEGERP (AREF (map, 0))
+ && XINT (AREF (map, 0)) <= op
+ && op - XINT (AREF (map, 0)) + 1 < ASIZE (map)))
{
reg[RRR] = -1;
break;
}
- size = ASIZE (map);
- point = XUINT (AREF (map, 0));
+ point = XINT (AREF (map, 0));
point = op - point + 1;
reg[RRR] = 0;
- if ((size <= 1) ||
- (!((point >= 1) && (point < size))))
+ content = AREF (map, point);
+ if (NILP (content))
reg[RRR] = -1;
- else
+ else if (INTEGERP (content))
+ reg[rrr] = XINT (content);
+ else if (EQ (content, Qt));
+ else if (CONSP (content))
{
- reg[RRR] = 0;
- content = AREF (map, point);
- if (NILP (content))
- reg[RRR] = -1;
- else if (NUMBERP (content))
- reg[rrr] = XINT (content);
- else if (EQ (content, Qt));
- else if (CONSP (content))
- {
- attrib = XCAR (content);
- value = XCDR (content);
- if (!NUMBERP (attrib) || !NUMBERP (value))
- continue;
- reg[rrr] = XUINT(value);
- break;
- }
- else if (SYMBOLP (content))
- CCL_CALL_FOR_MAP_INSTRUCTION (content, ic);
- else
- reg[RRR] = -1;
+ attrib = XCAR (content);
+ value = XCDR (content);
+ if (!INTEGERP (attrib) || !INTEGERP (value))
+ continue;
+ reg[rrr] = XINT(value);
+ break;
}
+ else if (SYMBOLP (content))
+ CCL_CALL_FOR_MAP_INSTRUCTION (content, ic);
+ else
+ reg[RRR] = -1;
}
break;
owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:bug#8719; Package emacs.
(Mon, 23 May 2011 07:28:01 GMT) Full text and rfc822 format available.Message #8 received at 8719 <at> debbugs.gnu.org (full text, mbox):
From: Kenichi Handa <handa <at> m17n.org> To: Paul Eggert <eggert <at> cs.ucla.edu> Cc: 8719 <at> debbugs.gnu.org Subject: Re: bug#8719: ccl: add some integer overflow checks Date: Mon, 23 May 2011 16:27:31 +0900
In article <4DDA0467.4050205 <at> cs.ucla.edu>, Paul Eggert <eggert <at> cs.ucla.edu> writes: > I did a quick pass through ccl.c and found several places where > integer overflow could cause problems. Here's a proposed fix for some > of the problems. > Does anybody have a good suggestion for testing these? I'm no > expert in CCL. Thank you. I'll check it (perhaps within this week). --- Kenichi Handa handa <at> m17n.org
owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:bug#8719; Package emacs.
(Mon, 23 May 2011 07:41:01 GMT) Full text and rfc822 format available.Message #11 received at 8719 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Paul Eggert <eggert <at> cs.ucla.edu>, Kenichi Handa <handa <at> m17n.org> Cc: 8719 <at> debbugs.gnu.org Subject: Re: bug#8719: ccl: add some integer overflow checks Date: Mon, 23 May 2011 03:40:14 -0400
> Date: Sun, 22 May 2011 23:53:27 -0700 > From: Paul Eggert <eggert <at> cs.ucla.edu> > > Does anybody have a good suggestion for testing these? I'm no > expert in CCL. We pretty much never use CCL these days, at least in Emacs packages. My reading of lisp/language/ethiopic.el is that it uses CCL to encode characters into font codepoints. So displaying some Ethiopic text (with an Ethiopic font installed) should run a CCL program. Perhaps Handa-san could suggest more tests.
Paul Eggert <eggert <at> cs.ucla.edu>:Paul Eggert <eggert <at> cs.ucla.edu>:Message #16 received at 8719-done <at> debbugs.gnu.org (full text, mbox):
From: Paul Eggert <eggert <at> cs.ucla.edu> To: 8722-done <at> debbugs.gnu.org, 8719-done <at> debbugs.gnu.org, 8668-done <at> debbugs.gnu.org Subject: fixes committed to trunk Date: Fri, 27 May 2011 13:40:10 -0700
I just committed bzr 104390 to the Emacs trunk. It merges fixes for bugs 8668, 8719, and 8722 as previously discussed. For Bug#8719, although the patch should fix the problems it may not be the best patch. Kenichi Handa wrote that he'll check it. Kenichi, if there are problems with it, please feel free to replace it with something better or to send me email with suggestions and I'll work on making it better. Thanks.
owner <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org:bug#8719; Package emacs.
(Tue, 31 May 2011 02:17:02 GMT) Full text and rfc822 format available.Message #19 received at 8719 <at> debbugs.gnu.org (full text, mbox):
From: Paul Eggert <eggert <at> cs.ucla.edu> To: Kazuhiro Ito <kzhr <at> d1.dion.ne.jp> Cc: 8719 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>, 8751-done <at> debbugs.gnu.org Subject: Re: bug#8751: 24.0.50; CCL is broken Date: Mon, 30 May 2011 19:16:40 -0700
> Here is a quick fix for actual code, but I don't check whether there > is other problems. Thanks, I came up with something similar independently, and installed a fix into the trunk (bzr 104441). This fix adjusted the comment as best I could. Bug#8719 and Bug#8751 are related so I'll CC: this to bug 8719 to give Kenichi Handa a heads-up.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Tue, 28 Jun 2011 11:24:04 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.