summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--ChangeLog6
-rw-r--r--generic/tclStringObj.c33
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);
}
}