Skip to content

Commit 5dd2b06

Browse files
authored
[clang-tidy] Fix OOB access in FormatStringConverter with signed chars (#169215)
`FormatStringConverter::appendFormatText` incorrectly treated non-ASCII characters (e.g. UTF-8) as negative values when using signed chars. This caused them to pass the `< 32` check for control characters. The negative values were passed to `llvm::hexdigit`, resulting in an OOB access and a crash. This closes [#169198](#169198)
1 parent 9bae84b commit 5dd2b06

File tree

4 files changed

+22
-6
lines changed

4 files changed

+22
-6
lines changed

clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,7 @@ void FormatStringConverter::finalizeFormatText() {
700700
/// Append literal parts of the format text, reinstating escapes as required.
701701
void FormatStringConverter::appendFormatText(const StringRef Text) {
702702
for (const char Ch : Text) {
703+
const auto UCh = static_cast<unsigned char>(Ch);
703704
if (Ch == '\a')
704705
StandardFormatString += "\\a";
705706
else if (Ch == '\b')
@@ -724,10 +725,10 @@ void FormatStringConverter::appendFormatText(const StringRef Text) {
724725
} else if (Ch == '}') {
725726
StandardFormatString += "}}";
726727
FormatStringNeededRewriting = true;
727-
} else if (Ch < 32) {
728+
} else if (UCh < 32) {
728729
StandardFormatString += "\\x";
729-
StandardFormatString += llvm::hexdigit(Ch >> 4, true);
730-
StandardFormatString += llvm::hexdigit(Ch & 0xf, true);
730+
StandardFormatString += llvm::hexdigit(UCh >> 4, true);
731+
StandardFormatString += llvm::hexdigit(UCh & 0xf, true);
731732
} else
732733
StandardFormatString += Ch;
733734
}

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Potentially Breaking Changes
6969
- `CharTypdefsToIgnore` to `CharTypedefsToIgnore` in
7070
:doc:`bugprone-signed-char-misuse
7171
<clang-tidy/checks/bugprone/signed-char-misuse>`
72-
72+
7373
- Modified the custom message format of :doc:`bugprone-unsafe-functions
7474
<clang-tidy/checks/bugprone/unsafe-functions>` by assigning a special meaning
7575
to the character ``>`` at the start of the value of the option
@@ -394,7 +394,7 @@ Changes in existing checks
394394
<clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
395395
an additional matcher that generalizes the copy-and-swap idiom pattern
396396
detection.
397-
397+
398398
- Improved :doc:`bugprone-unsafe-functions
399399
<clang-tidy/checks/bugprone/unsafe-functions>` check by hiding the default
400400
suffix when the reason starts with the character `>` in the `CustomFunctions`
@@ -497,7 +497,8 @@ Changes in existing checks
497497
- Improved :doc:`modernize-use-std-print
498498
<clang-tidy/checks/modernize/use-std-print>` check to correctly match
499499
when the format string is converted to a different type by an implicit
500-
constructor call.
500+
constructor call, and fixed a crash when handling format strings
501+
containing non-ASCII characters.
501502

502503
- Improved :doc:`performance-unnecessary-copy-initialization
503504
<clang-tidy/checks/performance/unnecessary-copy-initialization>` by printing

clang-tools-extra/test/clang-tidy/check_clang_tidy.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,8 @@ def parse_arguments() -> Tuple[argparse.Namespace, List[str]]:
398398

399399

400400
def main() -> None:
401+
sys.stdout.reconfigure(encoding="utf-8")
402+
sys.stderr.reconfigure(encoding="utf-8")
401403
args, extra_args = parse_arguments()
402404

403405
abbreviated_stds = args.std

clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ void printf_deceptive_newline() {
5454
// CHECK-FIXES: std::println("Hello");
5555
}
5656

57+
void printf_utf8_text() {
58+
printf("你好世界\n");
59+
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print]
60+
// CHECK-FIXES: std::println("你好世界");
61+
}
62+
5763
void printf_crlf_newline() {
5864
printf("Hello\r\n");
5965
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
@@ -303,6 +309,12 @@ void fprintf_simple() {
303309
// CHECK-FIXES: std::print(stderr, "Hello");
304310
}
305311

312+
void fprintf_utf8_text() {
313+
fprintf(stderr, "你好世界\n");
314+
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'fprintf' [modernize-use-std-print]
315+
// CHECK-FIXES: std::println(stderr, "你好世界");
316+
}
317+
306318
void std_printf_simple() {
307319
std::printf("std::Hello");
308320
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]

0 commit comments

Comments
 (0)