[core] prevent integer overflow when calling Align

Author: ferdymercury

core/base/inc/TString.h

@@ -230,7 +230,16 @@ friend std::strong_ordering operator<=>(const TString &s1, const TString &s2) {
 
    enum { kAlignment = 16 };
    static Ssiz_t  Align(Ssiz_t s) { return (s + (kAlignment-1)) & ~(kAlignment-1); }
-   static Ssiz_t  Recommend(Ssiz_t s) { return (s < kMinCap ? kMinCap : Align(s+1)) - 1; }
+   // `s` is expected to be <= MaxSize() = (kMaxInt-1) ; `s + 1` includes the terminating nullchar
+   static Ssiz_t  Recommend(Ssiz_t s)
+   {
+      if (s < kMinCap)
+         return kMinCap;
+      else if (s > MaxSize() - (kAlignment - 1))
+         return s;
+      else
+         return Align(s + 1) - 1;
+   }
    static Ssiz_t  AdjustCapacity(Ssiz_t oldCap, Ssiz_t newCap);
 
 private:
@@ -255,9 +264,9 @@ friend std::strong_ordering operator<=>(const TString &s1, const TString &s2) {
    char          *GetPointer() { return IsLong() ? GetLongPointer() : GetShortPointer(); }
    const char    *GetPointer() const { return IsLong() ? GetLongPointer() : GetShortPointer(); }
 #ifdef R__BYTESWAP
-   static Ssiz_t  MaxSize() { return kMaxInt - 1; }
+   static constexpr Ssiz_t MaxSize() { return kMaxInt - 1; }
 #else
-   static Ssiz_t  MaxSize() { return (kMaxInt >> 1) - 1; }
+   static constexpr Ssiz_t MaxSize() { return (kMaxInt >> 1) - 1; }
 #endif
    void           UnLink() const { if (IsLong()) delete [] fRep.fLong.fData; }
    void           Zero() {

core/base/src/TString.cxx

@@ -23,6 +23,14 @@ strings (<15 on 64-bit and <11 on 32-bit) are contained in the
 TString internal data structure without the need for mallocing the
 required space.
 
+\note TString can store a maximum of MaxSize()=2147483646 characters; ie 2147483647 bytes if you include the terminating null.
+Trying to allocate larger buffers might throw std::bad_alloc or raise
+Fatal errors or lead to undefined behavior. Likewise, there is no safety
+check if you pass a Long64_t to the class functions, they will be silently
+rounded to Int_t and lead to an integer overflow (negative value).
+For future designs, consider using std::string instead, which has a larger
+maximum size.
+
 Substring operations are provided by the TSubString class, which
 holds a reference to the original string and its data, along with
 the offset and length of the substring. To retrieve the substring
@@ -1220,12 +1228,11 @@ void TString::AssertElement(Ssiz_t i) const
 Ssiz_t TString::AdjustCapacity(Ssiz_t oldCap, Ssiz_t newCap)
 {
    Ssiz_t ms = MaxSize();
-   if (newCap > ms - 1) {
+   if (newCap > ms) {
       Fatal("TString::AdjustCapacity", "capacity too large (%d, max = %d)",
             newCap, ms);
    }
-   Ssiz_t cap = oldCap < ms / 2 - kAlignment ?
-                Recommend(std::max(newCap, 2 * oldCap)) : ms - 1;
+   Ssiz_t cap = oldCap <= ms / 2 ? Recommend(std::max(newCap, 2 * oldCap)) : ms;
    return cap;
 }