* generic/tclStringObj.c: Added overflow protections to the

        AppendUtfToUtfRep routine to either avoid invalid arguments and
        crashes, or to replace them with controlled panics.  [Bug 2561794]

2 files changed, 35 insertions, 4 deletions

diff --git a/ChangeLog b/ChangeLog
index 74b18bd..a5c16cc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-02-05  Don Porter  <dgp@users.sourceforge.net>
+
+	* generic/tclStringObj.c: Added overflow protections to the
+	AppendUtfToUtfRep routine to either avoid invalid arguments and
+	crashes, or to replace them with controlled panics.  [Bug 2561794]
+
 2009-02-04  Don Porter  <dgp@users.sourceforge.net>
 
 	* generic/tclStringObj.c (SetUnicodeObj):       Corrected failure of
diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c
index 0457b72..e7c8aae 100644
--- a/generic/tclStringObj.c
+++ b/generic/tclStringObj.c
@@ -33,7 +33,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tclStringObj.c,v 1.70.2.5 2009/02/04 21:43:51 dgp Exp $ */
+ * RCS: @(#) $Id: tclStringObj.c,v 1.70.2.6 2009/02/05 13:59:20 dgp Exp $ */
 
 #include "tclInt.h"
 #include "tommath.h"
@@ -727,6 +727,14 @@ Tcl_SetObjLength(
 {
     String *stringPtr;
 
+    if (length < 0) {
+	/*
+	 * Setting to a negative length is nonsense.  This is probably the
+	 * result of overflowing the signed integer range.
+	 */
+	Tcl_Panic("Tcl_SetObjLength: negative length requested: "
+		"%d (integer overflow?)", length);
+    }
     if (Tcl_IsShared(objPtr)) {
 	Tcl_Panic("%s called with shared object", "Tcl_SetObjLength");
     }
@@ -738,7 +746,7 @@ Tcl_SetObjLength(
      * Check that we're not extending a pure unicode string.
      */
 
-    if (length > (int) stringPtr->allocated &&
+    if ((size_t)length > stringPtr->allocated &&
 	    (objPtr->bytes != NULL || stringPtr->hasUnicode == 0)) {
 	/*
 	 * Not enough space in current string. Reallocate the string space and
@@ -841,6 +849,13 @@ Tcl_AttemptSetObjLength(
 {
     String *stringPtr;
 
+    if (length < 0) {
+	/*
+	 * Setting to a negative length is nonsense.  This is probably the
+	 * result of overflowing the signed integer range.
+	 */
+	return 0;
+    }
     if (Tcl_IsShared(objPtr)) {
 	Tcl_Panic("%s called with shared object", "Tcl_AttemptSetObjLength");
     }
@@ -1454,6 +1469,9 @@ AppendUtfToUtfRep(
      */
 
     oldLength = objPtr->length;
+    if (numBytes > INT_MAX - oldLength) {
+	Tcl_Panic("max size for a Tcl value (%d bytes) exceeded", INT_MAX);
+    }
     newLength = numBytes + oldLength;
 
     stringPtr = GET_STRING(objPtr);
@@ -1467,8 +1485,15 @@ AppendUtfToUtfRep(
 	 */
 
 	if (Tcl_AttemptSetObjLength(objPtr, 2 * newLength) == 0) {
-	    Tcl_SetObjLength(objPtr,
-		    newLength + numBytes + TCL_GROWTH_MIN_ALLOC);
+	    /*
+	     * Take care computing the amount of modest growth to avoid
+	     * overflow into invalid argument values for Tcl_SetObjLength.
+	     */
+	    unsigned int limit = INT_MAX - newLength;
+	    unsigned int extra = numBytes + TCL_GROWTH_MIN_ALLOC;
+	    int growth = (int) ((extra > limit) ? limit : extra);
+	
+	    Tcl_SetObjLength(objPtr, newLength + growth);
 	}
     }