Menu
▾
▴
#3685 Buggy string handling in command traces?
Looking at TEOv and TEOvI, trying to follow the trace code, I think I found a few possible bugs related to the 'command' string (no demos yet).
(1) TEOv only generates a 'command' string from objv when there are interp-wide traces, but fails to do so on traces on the specific command. This is needed later on, especially since traces use the string interface. Note that this hints at moving the string generation to TEOvI, after Tcl_GetCommandFromObj has happened.
(2) TclCheckInterpTraces and TclCheckExecutionTraces accept a counted (not nul-terminated) string for the 'command', but pass on just the char* and not the count. The receiver has no way to avoid a buffer overflow in case the string was not nul-terminated. Both TEBC and Tcl_Eval call TEOvI with counted strings (from the bc source resp from a parse token), so that the problem is not just theoretic.
I see one possible solution:
(a) generate nul-terminated strings for the traces, with better detection of when it is needed. This
should happen in TCIT/TCET. I am not so worried of the duplication of effort in case both interp-wide and command-specific traces are in place
(b) modify the api so that the user generates a string itself from objc/objv if none is supplied. This could well happen in TCIT and TCET, instead of burdening the callers to do so.
Candidate patch forthcoming.
Logged In: YES
user_id=148712
Originator: YES
Patch attached
File Added: trace.diff
Logged In: YES
user_id=148712
Originator: YES
Specifically, assume a command with execution traces called from TEBC or Tcl_Eval, hence with a non nul-terminated 'command' string.
TEOvI will call TclCheckExecutionTraces, passing along the string as is. In turn, this calls TraceExecutionProc, passing along the string but *not* the count.
Finally, TraceExecutionProc uses it for strcmp (line 1777), strlen and strcpy (lines 1893-4).
Actually, 'command' will typically be nul-terminated, just not at the end of the command (at count), but rather at the end of the script. So: no buffer overflow, just a too-lengthy string will be used. Reducing prio.
Logged In: YES
user_id=148712
Originator: YES
Fixed as part of larger commit - same idea, different patch.