Tcl Source Code

View Ticket
Login
Ticket UUID: 2380293
Title: binary decode uuencode foo --> segfault
Type: Bug Version: obsolete: 8.6a4
Submitter: duoas Created on: 2008-12-03 00:02:04
Subsystem: 12. ByteArray Object Assigned To: patthoyts
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2008-12-16 00:15:34
Resolution: Fixed Closed By: ferrieux
    Closed on: 2008-12-15 17:15:34
Description:
Yep, trying to decode an invalid string causes a segfault (meaning Tcl crashes and burns).

To replicate, start up tclsh86 and type:

  binary decode uuencode foo

Hope this helps.
User Comments: ferrieux added on 2008-12-16 00:15:33:
After a chat with Kevin, only answerer, it appears that non-strictness has an obvious usecase with randomly interspersed whitespace. That was the perturbation process I was after.
But it is also clear that any other kind of unexpected character is fishy and should raise an error.
Just committed this change:
 - fix the off-by-one bug that caused crashes or truncation (which is not 4/3 but only of one char at maximum, because it can occur only in the last 4-byte chunk)
 - redefine the non-strict contract to allow only whitespace as defined by the isspace() function.

ferrieux added on 2008-12-12 15:47:08:
OK now I know the algorithm. The off-by-one is the visible part of a larger iceberg: the "cut" variable counts missing bytes in the source (base 64) stream, and is wrongly translated 1-to-1 into missing bytes in the decoded stream. So there's a systematic error of factor 4/3 (since a "cut++" should only count as removing 6 bits).

However, I believe the very principle of allowing non-strict uudecoding is flawed. Indeed, a "BadUU" byte in the middle is just skipped, but I wonder about the perturbation process that this is modelling. Indeed if random bytes are inserted here and there, only those which are fortunately outside the 32..96 range will be filtered out. So I cannot imagine a practical situation where an uuencoded stream would be decoded non-strictly... So the quick patch is enough to avoid crashes, the only remaining error being truncated results for error-filled inputs.

Pat, your feeling ?

ferrieux added on 2008-12-12 07:11:01:

File Added - 304973: uu.patch

This is due to an off-by-one length computation yielding -1 in a non-strict case.
Knowing nothing of the uuencode algorithm I cannot fix it properly; however, the attached patch fixes the crash by saturating the size to nonnegative values.
File Added: uu.patch

Attachments: