Summary
AString / UString in src/CPP/Common/MyString.cpp appear to have aliasing hazards in mutating paths that may reallocate before copying from the source pointer.
I ran into this while reviewing downstream CodeQL findings against the 1.5.1 code currently pinned in another project.
Why this looks unsafe
AString and UString expose pointers into their own storage via APIs like:
operator const char *() const
const char *Ptr() const
const char *Ptr(unsigned pos) const
operator const wchar_t *() const
const wchar_t *Ptr() const
const wchar_t *Ptr(unsigned pos) const
That means callers can legally pass a pointer into the string's current buffer back into mutating functions.
In several mutators, the code can do this sequence:
- detect
len > _limit
- allocate a new buffer
- delete the old
_chars
- copy from the source pointer
s
If s aliases the old buffer, the copy reads freed memory.
Concrete examples
AString::operator=(const char *s)
unsigned len = MyStringLen(s);
if (len > _limit)
{
char *newBuf = MY_STRING_NEW_char((size_t)len + 1);
MY_STRING_DELETE(_chars)
_chars = newBuf;
_limit = len;
}
_len = len;
MyStringCopy(_chars, s);
AString::SetFrom(const char *s, unsigned len)
if (len > _limit)
{
char *newBuf = MY_STRING_NEW_char((size_t)len + 1);
MY_STRING_DELETE(_chars)
_chars = newBuf;
_limit = len;
}
if (len != 0)
memcpy(_chars, s, len);
UString::SetFrom(const wchar_t *s, unsigned len)
if (len > _limit)
{
wchar_t *newBuf = MY_STRING_NEW_wchar_t((size_t)len + 1);
MY_STRING_DELETE(_chars)
_chars = newBuf;
_limit = len;
}
if (len != 0)
wmemcpy(_chars, s, len);
Reproducer shape
The risky pattern is something like:
AString s("abcdef");
const char *tail = s.Ptr(2);
s.SetFrom(tail, 4); // source may alias old buffer
and similarly for UString.
If the implementation needs to grow first, tail can dangle before the copy.
Expected fix direction
Any path that may reallocate before copying from a caller-supplied pointer likely needs to either:
- detect aliasing with the current buffer and preserve the source first, or
- copy through a temporary before freeing the old storage, or
- restructure the growth path so the old buffer remains valid until the copy is complete.
I did not prepare a patch in this report; this is an upstream issue so maintainers can choose the preferred fix shape.
If useful, I can follow up with a minimal patch proposal limited to the aliasing cases.
Summary
AString/UStringinsrc/CPP/Common/MyString.cppappear to have aliasing hazards in mutating paths that may reallocate before copying from the source pointer.I ran into this while reviewing downstream CodeQL findings against the
1.5.1code currently pinned in another project.Why this looks unsafe
AStringandUStringexpose pointers into their own storage via APIs like:operator const char *() constconst char *Ptr() constconst char *Ptr(unsigned pos) constoperator const wchar_t *() constconst wchar_t *Ptr() constconst wchar_t *Ptr(unsigned pos) constThat means callers can legally pass a pointer into the string's current buffer back into mutating functions.
In several mutators, the code can do this sequence:
len > _limit_charssIf
saliases the old buffer, the copy reads freed memory.Concrete examples
AString::operator=(const char *s)AString::SetFrom(const char *s, unsigned len)UString::SetFrom(const wchar_t *s, unsigned len)Reproducer shape
The risky pattern is something like:
and similarly for
UString.If the implementation needs to grow first,
tailcan dangle before the copy.Expected fix direction
Any path that may reallocate before copying from a caller-supplied pointer likely needs to either:
I did not prepare a patch in this report; this is an upstream issue so maintainers can choose the preferred fix shape.
If useful, I can follow up with a minimal patch proposal limited to the aliasing cases.