Skip to content

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Feb 13, 2025

Reverts #127076 to reland #125020

Use-after-free should be fixed here #127063

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:as-a-library libclang and C++ API labels Feb 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 13, 2025

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

Reverts llvm/llvm-project#127076


Full diff: https://github.com/llvm/llvm-project/pull/127078.diff

2 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/tools/libclang/CXString.cpp (+1-13)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 03bddbe3e983a..e41ad384b84f7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -257,6 +257,9 @@ clang-format
 libclang
 --------
 
+- Fixed a buffer overflow in ``CXString`` implementation. The fix may result in
+  increased memory allocation.
+
 Code Completion
 ---------------
 
diff --git a/clang/tools/libclang/CXString.cpp b/clang/tools/libclang/CXString.cpp
index 5e427957a1092..aaa8f8eeb67a1 100644
--- a/clang/tools/libclang/CXString.cpp
+++ b/clang/tools/libclang/CXString.cpp
@@ -87,19 +87,7 @@ CXString createRef(StringRef String) {
   if (String.empty())
     return createEmpty();
 
-  // If the string is not nul-terminated, we have to make a copy.
-
-  // FIXME: This is doing a one past end read, and should be removed! For memory
-  // we don't manage, the API string can become unterminated at any time outside
-  // our control.
-
-  if (String.data()[String.size()] != 0)
-    return createDup(String);
-
-  CXString Result;
-  Result.data = String.data();
-  Result.private_flags = (unsigned) CXS_Unmanaged;
-  return Result;
+  return createDup(String);
 }
 
 CXString createDup(StringRef String) {

@vitalybuka vitalybuka marked this pull request as draft February 13, 2025 15:43
@vitalybuka vitalybuka marked this pull request as ready for review February 19, 2025 06:43
@vitalybuka vitalybuka added the skip-precommit-approval PR for CI feedback, not intended for review label Feb 19, 2025
Created using spr 1.3.4
@vitalybuka
Copy link
Collaborator Author

@vitalybuka vitalybuka merged commit a16fa3a into main Feb 20, 2025
11 checks passed
@vitalybuka vitalybuka deleted the revert-127076-revert-125020-users/vitalybuka/spr/libclang-always-dup-in-createref branch February 20, 2025 02:41
@vitalybuka
Copy link
Collaborator Author

@fmayer Looks like my patch exposed another UAF
https://lab.llvm.org/buildbot/#/builders/169/builds/8637

Looking, and will revert if there is no quick fix

vitalybuka added a commit that referenced this pull request Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:as-a-library libclang and C++ API clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants