| Ticket UUID: | 272e866f1ec0af1927a7899a81b1c58395832096 | |||
| Title: | Uncontrolled overflow in ReadBytes() | |||
| Type: | Bug | Version: | 8.5.15 | |
| Submitter: | gustafn | Created on: | 2013-11-12 19:05:42 | |
| Subsystem: | 25. Channel System | Assigned To: | dgp | |
| Priority: | 8 | Severity: | Severe | |
| Status: | Closed | Last Modified: | 2014-05-08 17:49:52 | |
| Resolution: | Fixed | Closed By: | dgp | |
| Closed on: | 2014-05-08 17:49:52 | |||
| Description: |
When reading a file with e.g. 1.1 GB via set content [read $f] Tcl crashes due to the doubling policy of reallocs on machines where sizeof(int) == 32 (which is as well the case on 64 bit Linux). Due to doubling the length of the buffer, the length variable of type "int" becomes negative on values larger than 1GB. While one can discuss the usefulness of reading large files into memory, the situation can be improved quite easily by limiting the doubling policy to 2GB (actually INT_MAX). The problem happened in a Tcl-based zip-file generator, when the size of a single file is larger than 1.x GB. A sample patch is available (I assume i can attach the patch after writing the ticket). Most probably there are more places in Tcl, where a similar patch might be useful. | |||
| User Comments: |
dgp added on 2014-05-08 17:49:52:
This is fixed on trunk and core-8-5-branch now. gustafn added on 2014-04-10 14:35:37: We tested the the patches on the mentioned branch in tcl8.5, and everything seems to work fine. We had to copy/paste manually some macro definitions on generic/tclInt.h and the function TclAppendBytesToByteArray which were introduced on tcl 8.6. dkf added on 2014-01-16 22:42:43: Patch seems reasonable, but removing the stuff currently elided with #if 0 is better practice. We have fossil to keep the history of bad ideas for us now! dgp added on 2014-01-15 20:22:13: See branch bug-272e866f1e for a possible fix. gustafn added on 2013-12-09 15:28:16: yep, using Tcl_AttemptSetObjLength() seems better, although the function is quite complex. i would prefer a function Tcl_GrowObjLength(... desired_length, required_min_length) to keep the changes mininmal and to encapsulate the growing behavior. gustafn added on 2013-12-09 15:27:53: yep, using Tcl_AttemptSetObjLength() seems better, although the function is quite complex. i would prefer a function Tcl_GrowObjLength(... desired_length, required_min_length) to keep the changes mininmal and to encapsulate the growing behavior. dgp added on 2013-12-03 15:44:26: The fix for the analogous problem in ReadChars() was made in checkin http://core.tcl.tk/tcl/info/cdf00220922c7d8d so it's been good since 8.5.8. The ticket covering the development of that change is http://core.tcl.tk/tcl/info/2107634 Looks like we just need the analogous fix put in place for binary channels. dgp added on 2013-12-03 15:24:52: We've tidied up many overflow scenarios but apparently not this one. Similar trouble may lurk in ReadChars() dkf added on 2013-11-14 22:27:26: dgp has worked on limits… | |||
Attachments:
- tclI-2GB.patch [download] added by gustafn on 2013-11-12 19:07:45. [details]