From 9d3c4057d313047c7da3fa8d9624b06913b814d1 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 5 Feb 2009 13:59:20 +0000 Subject: * 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] --- ChangeLog | 6 ++++++ generic/tclStringObj.c | 33 +++++++++++++++++++++++++++++---- 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 + + * 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 * 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); } } -- cgit v0.12