* bug#8719: ccl: add some integer overflow checks
@ 2011-05-23 6:53 Paul Eggert
2011-05-23 7:27 ` Kenichi Handa
2011-05-23 7:40 ` Eli Zaretskii
0 siblings, 2 replies; 3+ messages in thread
From: Paul Eggert @ 2011-05-23 6:53 UTC (permalink / raw)
To: 8719
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#8719: ccl: add some integer overflow checks
2011-05-23 6:53 bug#8719: ccl: add some integer overflow checks Paul Eggert
@ 2011-05-23 7:27 ` Kenichi Handa
2011-05-23 7:40 ` Eli Zaretskii
1 sibling, 0 replies; 3+ messages in thread
From: Kenichi Handa @ 2011-05-23 7:27 UTC (permalink / raw)
To: Paul Eggert; +Cc: 8719
In article <4DDA0467.4050205@cs.ucla.edu>, Paul Eggert <eggert@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@m17n.org
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#8719: ccl: add some integer overflow checks
2011-05-23 6:53 bug#8719: ccl: add some integer overflow checks Paul Eggert
2011-05-23 7:27 ` Kenichi Handa
@ 2011-05-23 7:40 ` Eli Zaretskii
1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2011-05-23 7:40 UTC (permalink / raw)
To: Paul Eggert, Kenichi Handa; +Cc: 8719
> Date: Sun, 22 May 2011 23:53:27 -0700
> From: Paul Eggert <eggert@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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-05-23 7:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-23 6:53 bug#8719: ccl: add some integer overflow checks Paul Eggert
2011-05-23 7:27 ` Kenichi Handa
2011-05-23 7:40 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).