Skip to content

Commit 8685dda

Browse files
authored
Fix crashes caused by a race condition in icu::OlsonTimeZone::initTransitionRules (#1597)
ICU's `UTimeZone` isn't actually thread safe. Lock around the access. I was able to reproduce the UAF crash by calling uatimezone API concurrently. With this fix, I no longer reproduce the crash. 164388304
1 parent d8533a2 commit 8685dda

File tree

1 file changed

+51
-33
lines changed

1 file changed

+51
-33
lines changed

Sources/FoundationInternationalization/TimeZone/TimeZone_ICU.swift

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
4545
}
4646

4747
// This is safe because it's only mutated at deinit time
48-
nonisolated(unsafe) private let _timeZone : UnsafePointer<UTimeZone?>
48+
nonisolated(unsafe) private let _timeZone : LockedState<UnsafePointer<UTimeZone?>>
4949

5050
// This type is safely sendable because it is guarded by a lock in _TimeZoneICU and we never vend it outside of the lock so it can only ever be accessed from within the lock
5151
struct State : @unchecked Sendable {
@@ -86,8 +86,10 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
8686
ucal_close(c)
8787
}
8888

89-
let mutableT = UnsafeMutablePointer(mutating: _timeZone)
90-
uatimezone_close(mutableT)
89+
_timeZone.withLock {
90+
let mutableT = UnsafeMutablePointer(mutating: $0)
91+
uatimezone_close(mutableT)
92+
}
9193
}
9294

9395
required init?(identifier: String) {
@@ -117,8 +119,7 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
117119
guard let timeZone else {
118120
return nil
119121
}
120-
121-
self._timeZone = UnsafePointer(timeZone)
122+
self._timeZone = .init(initialState:timeZone)
122123
self.name = name
123124
lock = LockedState(initialState: State())
124125
}
@@ -133,15 +134,16 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
133134
}
134135

135136
func secondsFromGMT(for date: Date) -> Int {
136-
var rawOffset: Int32 = 0
137-
var dstOffset: Int32 = 0
138-
var status: UErrorCode = U_ZERO_ERROR
139-
uatimezone_getOffset(_timeZone, date.udate, 0, &rawOffset, &dstOffset, &status)
140-
guard status.checkSuccessAndLogError("error getting uatimezone offset") else {
141-
return 0
137+
return _timeZone.withLock {
138+
var rawOffset: Int32 = 0
139+
var dstOffset: Int32 = 0
140+
var status: UErrorCode = U_ZERO_ERROR
141+
uatimezone_getOffset($0, date.udate, 0, &rawOffset, &dstOffset, &status)
142+
guard status.checkSuccessAndLogError("error getting uatimezone offset") else {
143+
return 0
144+
}
145+
return Int((rawOffset + dstOffset) / 1000)
142146
}
143-
144-
return Int((rawOffset + dstOffset) / 1000)
145147
}
146148

147149
func abbreviation(for date: Date) -> String? {
@@ -157,33 +159,38 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
157159
}
158160

159161
func daylightSavingTimeOffset(for date: Date) -> TimeInterval {
160-
var rawOffset_unused: Int32 = 0
161-
var dstOffset: Int32 = 0
162-
var status = U_ZERO_ERROR
163-
uatimezone_getOffset(_timeZone, date.udate, 0, &rawOffset_unused, &dstOffset, &status)
164-
if status.isSuccess {
162+
_timeZone.withLock {
163+
var rawOffset_unused: Int32 = 0
164+
var dstOffset: Int32 = 0
165+
var status = U_ZERO_ERROR
166+
uatimezone_getOffset($0, date.udate, 0, &rawOffset_unused, &dstOffset, &status)
167+
guard status.isSuccess else {
168+
return 0.0
169+
}
165170
return TimeInterval(Double(dstOffset) / 1000.0)
166-
} else {
167-
return 0.0
168171
}
169172
}
170173

171174
func nextDaylightSavingTimeTransition(after date: Date) -> Date? {
172-
var status = U_ZERO_ERROR
173-
var answer = UDate(0.0)
174-
let success = uatimezone_getTimeZoneTransitionDate(_timeZone, date.udate, UCAL_TZ_TRANSITION_NEXT, &answer, &status)
175+
let limit = Date.validCalendarRange.upperBound
176+
let answer: UDate? = _timeZone.withLock {
177+
var status = U_ZERO_ERROR
178+
var answer = UDate(0.0)
179+
let success = uatimezone_getTimeZoneTransitionDate($0, date.udate, UCAL_TZ_TRANSITION_NEXT, &answer, &status)
180+
guard (success != 0) && status.isSuccess && answer < limit.udate else {
181+
return nil
182+
}
183+
return answer
184+
}
175185

176-
let limit = Date.validCalendarRange.upperBound
177-
guard (success != 0) && status.isSuccess && answer < limit.udate else {
178-
return nil
179-
}
180-
return Date(udate: answer)
181-
}
186+
guard let answer else {
187+
return nil
188+
}
189+
190+
return Date(udate: answer)
191+
}
182192

183193
func rawAndDaylightSavingTimeOffset(for date: Date, repeatedTimePolicy: TimeZone.DaylightSavingTimePolicy = .former, skippedTimePolicy: TimeZone.DaylightSavingTimePolicy = .former) -> (rawOffset: Int, daylightSavingOffset: TimeInterval) {
184-
var rawOffset: Int32 = 0
185-
var dstOffset: Int32 = 0
186-
var status = U_ZERO_ERROR
187194
let icuDuplicatedTime: UTimeZoneLocalOption
188195
switch repeatedTimePolicy {
189196
case .former:
@@ -200,7 +207,18 @@ internal final class _TimeZoneICU: _TimeZoneProtocol, Sendable {
200207
icuSkippedTime = UCAL_TZ_LOCAL_LATTER
201208
}
202209

203-
uatimezone_getOffsetFromLocal(_timeZone, icuSkippedTime, icuDuplicatedTime, date.udate, &rawOffset, &dstOffset, &status)
210+
let (rawOffset, dstOffset): (Int32, Int32) = _timeZone.withLock {
211+
var rawOffset: Int32 = 0
212+
var dstOffset: Int32 = 0
213+
var status = U_ZERO_ERROR
214+
uatimezone_getOffsetFromLocal($0, icuSkippedTime, icuDuplicatedTime, date.udate, &rawOffset, &dstOffset, &status)
215+
216+
guard status.isSuccess else {
217+
return (0, 0)
218+
}
219+
return (rawOffset, dstOffset)
220+
}
221+
204222
return (Int(rawOffset / 1000), TimeInterval(dstOffset / 1000))
205223
}
206224

0 commit comments

Comments
 (0)