apple
Calling Date.description might result in undefined behaviour
Date’s description function uses a fixed size buffer (26 bytes, see here). This buffer is given to strftime.
Both, dates before “-999-01-01 00:00:00 +0000” and starting at “10000-01-01 00:00:00 +0000”, will require 27 bytes or more (including null byte at the end).
The return value of this function is checked by guard strftime(…) >= 0 (see). While most library functions in C return a negative value on failure, strftime returns 0 if the buffer is not big enough to hold the formatted date. Thus this guard never executes.
The buffer in this case is not guaranteed to contain a ending null byte, as the man page states: “Otherwise, 0 shall be returned and the contents of the array are unspecified.” (Highlighting mine)
The undefined content (worst case: 26 bytes forming valid UTF-8, e.g. 26 spaces) is then given to String(validatingUTF8:) which probably will (in the worst case) run behind the end of the buffer, resulting in a crash.
I suggest to either replace the call to strftime by a better API or change the buffer size to be big enough to hold any description of any point in time that Date might represent. In addition to that, the guard should be changed to guard strftime(…) >= 25 given that is the smallest valid size.
test cases:
Date(timeIntervalSinceReferenceDate: TimeInterval(-63145526400)).description == "0000-01-01 00:00:00 +0000"
Date(timeIntervalSinceReferenceDate: TimeInterval(-4200000000000)).description == "-131090-12-30 21:20:00 +0000"
Date(timeIntervalSinceReferenceDate: TimeInterval(4200000000000)).description == "15310-04-10 02:40:00 +0000"
I'd rather start with 26 bytes like we do now and allow for one single reallocation if strftime returns 0 - which is guaranteed (in our specific case / given the format) to be due to undersizing the buffer.
I slightly disagree with guard strftime(…) >= 25 (although true in practice), but it should not be required if we're looking for 0 directly (as per man).
The memory is deallocated at the end of the block, why not use a few bytes more than the minimum? Thinking about that, I’d use something that just allocates the buffer on the stack:
lazy var unavailable = { "<description unavailable>" } ()
let format = "%Y-%m-%d %H:%M:%S +0000"
let bufferSize = 512
var info = tm()
var time = time_t(self.timeIntervalSince1970)
gmtime_r(&time, &info)
return withUnsafeTemporaryAllocation(of: CChar.self, capacity: bufferSize) { buffer -> String in
guard let ptr = buffer.baseAddress else {
return unavailable
}
guard strftime(ptr, bufferSize, format, &info) != 0 else {
return unavailable
}
guard let result = String(validatingUTF8: ptr) else {
return unavailable
}
return result
}
I know, 512 bytes is unreasonable.
On the other hand, currently the compiler considers up to 1024 bytes as small enough to live on the stack (see).
It could be whatever the is used at most (with the given max time_t) but I think, I’d just use 64 or 128 and it will work with every strange architecture even when time_t is way bigger.
Looks solid 🚀. Out of curiosity, why not let unavailable instead (since we're dealing with a string literal anyway)?
My only argument against a large initial buffer was that in 99.999% of the cases the buffer is exactly 26 bytes. That being said, you're right, there's literally no gain to be had, so we might as well allocate 512.
Looks solid 🚀. Out of curiosity, why not
let unavailableinstead (since we're dealing with a string literal anyway)?
I was over-optimizing. (I thought that assigning the string literal to the variable would require a copy of the string.)
