Subject: ccl: add some integer overflow checks

From: Paul Eggert <[email-masked]>
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;