Skip to content

Commit a61b743

Browse files
authored
Potential stack buffer overflow in _withFixedCharBuffer (#672)
In `_withFixedCharBuffer` helper, we allocate a buffer with the requested size + 1 (for null-termination). With the assumption that the closure's return value `len <= the requested size`, the null-terminated character should be on index `len`, not `len + 1`. Fixes rdar://129154192
1 parent 654fa54 commit a61b743

File tree

3 files changed

+11
-2
lines changed

3 files changed

+11
-2
lines changed

Sources/FoundationInternationalization/ICU/ICU+Foundation.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ internal func _withResizingCharBuffer(initialSize: Int32 = 32, _ body: (UnsafeMu
114114
var innerStatus = U_ZERO_ERROR
115115
if let innerLen = body(innerBuffer.baseAddress!, len + 1, &innerStatus) {
116116
if innerStatus.isSuccess && innerLen > 0 {
117+
buffer[Int(innerLen)] = 0
117118
return String(validatingUTF8: innerBuffer.baseAddress!)
118119
}
119120
}
@@ -122,6 +123,7 @@ internal func _withResizingCharBuffer(initialSize: Int32 = 32, _ body: (UnsafeMu
122123
return nil
123124
}
124125
} else if status.isSuccess && len > 0 {
126+
buffer[Int(len)] = 0
125127
return String(validatingUTF8: buffer.baseAddress!)
126128
}
127129
}
@@ -137,7 +139,7 @@ internal func _withFixedCharBuffer(size: Int32 = ULOC_FULLNAME_CAPACITY + ULOC_K
137139
var status = U_ZERO_ERROR
138140
if let len = body(buffer.baseAddress!, size, &status) {
139141
if status.isSuccess && len > 0 {
140-
buffer[Int(len + 1)] = 0
142+
buffer[Int(len)] = 0
141143
return String(validatingUTF8: buffer.baseAddress!)
142144
}
143145
}

Sources/FoundationInternationalization/Locale/Locale_ICU.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1615,7 +1615,9 @@ extension Locale {
16151615
}
16161616

16171617
internal static func keywordValue(identifier: String, key: String) -> String? {
1618-
return _withFixedCharBuffer(size: ULOC_KEYWORD_AND_VALUES_CAPACITY) { buffer, size, status in
1618+
// Unlike other many ICU variables, `ULOC_KEYWORD_AND_VALUES_CAPACITY` does not include null-termination.
1619+
// Manually add one here.
1620+
return _withFixedCharBuffer(size: ULOC_KEYWORD_AND_VALUES_CAPACITY + 1) { buffer, size, status in
16191621
return uloc_getKeywordValue(identifier, key, buffer, size, &status)
16201622
}
16211623
}

Tests/FoundationInternationalizationTests/LocaleTests.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,11 @@ final class LocalePropertiesTests : XCTestCase {
581581

582582
verify("ar_EG@calendar=ethioaa;collation=dict;currency=frf;fw=fri;hours=h11;measure=uksystem;numbers=traditio;rg=uszzzz;sd=usca", expectedLanguage: "ar", script: "arab", languageRegion: "EG", region: "us", subdivision: "usca", measurementSystem: .uk, calendar: .ethiopicAmeteAlem, hourCycle: .zeroToEleven, currency: "FRF", numberingSystem: "traditio", numberingSystems: [ "traditio", "latn", "arab" ], firstDayOfWeek: .friday, collation: "dict")
583583
}
584+
585+
func test_longLocaleKeywordValues() {
586+
let x = Locale.keywordValue(identifier: "ar_EG@vt=kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk", key: "vt")
587+
XCTAssertEqual(x, "kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk")
588+
}
584589
}
585590

586591
// MARK: - Bridging Tests

0 commit comments

Comments
 (0)