Tcl Source Code

Security issue: memory corruption that a script in a safe interpreter can trigger on trunk and in every release, and that any trusted script can trigger everywhere. The C clock code takes the rows of ::tcl::clock::TZData as they are and indexes into them without checking their length. A row with fewer than four elements makes it read a Tcl_Obj * past the end of the list's element array, and then increment that object's reference count and use it as the time zone name. An empty row gives a NULL dereference.

All three PoCs below run in a plain tclsh. The first line loads the Tcl part of clock (needed on 8.6, harmless on 9.x).

1. UTC to local. ConvertUTCToLocalUsingTable reads cellv[3] (and cellv[1] for a 1-element row) without checking cellc:

   clock format 0 -gmt 1
   set ::tcl::clock::TZData(:bad) {{0 0}}
   clock format 10 -timezone :bad
   

   ASan on trunk:

   READ of size 8 ... is located 8 bytes after 56-byte region
   SUMMARY: AddressSanitizer: heap-buffer-overflow tclClock.c:2307 in ConvertUTCToLocalUsingTable
   

   The same happens on 9.1.0 (line 2307), 9.0.4 and core-9-0-branch (2316), 8.6.18 and core-8-6-branch (1023). With {0} as the row, the read is cellv[1], passed to TclGetIntFromObj (line 2299 on trunk, 1015 on 8.6).

2. Local to UTC (9.0 and later). ConvertLocalToUTCUsingTable has the same cellv[3] read. clock scan gets there when the scanned time falls into the short row:

   clock format 0 -gmt 1
   set ::tcl::clock::TZData(:bad) {{-9223372036854775808 0 0 A} {100000 0}}
   clock scan "1970-01-03 00:00:00" -format "%Y-%m-%d %T" -base 10 -timezone :bad
   
   SUMMARY: AddressSanitizer: heap-buffer-overflow tclClock.c:2043 in ConvertLocalToUTCUsingTable
   

   Line 2052 on 9.0.4 and core-9-0-branch. A 1-element row makes it read cellv[1] past the array (line 2031), on 8.6 as well (line 826); 8.6 has no cellv[3] read here.

3. Empty row. TclClockLookupLastTransition (LookupLastTransition on 8.6) takes element 0 of a row with Tcl_ListObjIndex, which returns NULL for an empty list, and passes it on to TclGetWideIntFromObj:

   clock format 0 -gmt 1
   set ::tcl::clock::TZData(:bad) {{}}
   clock format 10 -timezone :bad
   
   tclClock.c:2435:9: runtime error: member access within null pointer of type 'Tcl_Obj'
   

   Line 2444 on 9.0.4, 1153 on 8.6. This one is only a crash.

The stray pointer in 1 and 2 is more than a read: it gets a reference count increment, is stored in the conversion cache and in the result, and is read as a string, so whatever object or data it points to is written to and may later be freed. With the normal allocator the slot past a small list usually lies inside the same allocator block and holds leftover data from an earlier use. In one run on an optimised trunk build it held string bytes (0x206d756e67695364), and the script has a lot of influence over what is left there. Without sanitisers all three PoCs end in a segmentation fault on 9.1.0 and trunk.

Exploitability. Items 1 and 2 are assessed as plausibly exploitable for code execution, given an address leak or heap predictability. No exploit has been written. Item 3 is a denial of service only.

In a safe interpreter clock is an alias to the parent's, so the child's own TZData does not reach it that way. But on trunk and in all releases the ::tcl::clock::* helpers are visible in safe interps, and these take the child's own TZData:

interp eval [interp create -safe] {
    set ::tcl::clock::TZData(:bad) {{0 0}}
    ::tcl::clock::GetDateFields 10 :bad 2361222
}

This gives the same report as 1 on trunk, 9.1.0 and 9.0.4 (on 8.6 the helpers take the rows themselves instead of a name, with the same result).

The helpers should not be visible there at all. TclClockInit creates them from Tcl_CreateInterp and skips them only if Tcl_IsSafe(interp) is true, but a safe child is only marked safe by MakeSafe, after Tcl_CreateInterp has returned. So every safe interp gets them, and the same gap lets it read any environment variable of the process through ::tcl::clock::getenv. Both maintenance branches have fixed this: core-8-6-branch creates the child as safe from the start (940c0e06e1), and core-9-0-branch moves TclClockInit to Tcl_Init, which a safe child never runs, and reverts 2316f673af (a94e95399a). Neither fix is on trunk (both reached it only through merges without content) or in a release. The recommended fix is to bring one of them to trunk, together with the revert of 2316f673af, and to release 8.6.x and 9.0.x with them. That stops safe interps from reaching the bug in this ticket, but it does not fix the bug itself: trusted scripts still trigger it on every branch, so the checks below are needed as well.

The reads go back to the C clock code of 8.5.0 (2005), except the cellv[3] read in local to UTC, which came with the clock rewrite in 9.0.

Fix suggestion. In ConvertUTCToLocalUsingTable and ConvertLocalToUTCUsingTable, which read cellv[1] and cellv[3], check cellc >= 4 after TclListObjGetElements and return an error such as "malformed time zone data" otherwise. In TclClockLookupLastTransition, treat compObj == NULL from either Tcl_ListObjIndex call (first row and binary search) as the same error. 8.6 needs the same checks in its versions of the three functions. Checking each row once where TZData is fetched would cover every user of the table, at some cost per lookup.