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.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Paul Eggert <eggert <at> cs.ucla.edu> Subject: bug#8719: closed (fixes committed to trunk) Date: Fri, 27 May 2011 20:41:02 +0000
[Message part 1 (text/plain, inline)]
Your bug report #8719: ccl: add some integer overflow checks which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 8719 <at> debbugs.gnu.org. -- 8719: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8719 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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 -0700I 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.
[Message part 3 (message/rfc822, inline)]
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 -0700I 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;
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.