LibCore: Avoid buffer overrun when invoking crypt() with a SecretString

For example, consider the following SecretString construction:

    String foo = "foo";
    auto ss = SecretString::take_ownership(foo.to_byte_buffer());

The ByteBuffer created by to_byte_buffer() will not contain the NUL
terminator. Therefore, the value returned by SecretString::characters
will not be NUL-terminated either.

Currently, the only use of SecretString is to pass its character data to
crypt(), which requires a NUL-terminated string. To ensure this cannot
result in a buffer overrun, make SecretString append a NUL terminator to
its buffer if there isn't one already.
This commit is contained in:
Timothy Flynn 2021-10-19 09:14:05 -04:00 committed by Andreas Kling
parent 4739982c66
commit 204a091765

View file

@ -30,6 +30,13 @@ SecretString SecretString::take_ownership(ByteBuffer&& buffer)
SecretString::SecretString(ByteBuffer&& buffer)
: m_secure_buffer(move(buffer))
{
// SecretString is currently only used to provide the character data to invocations to crypt(),
// which requires a NUL-terminated string. To ensure this operation avoids a buffer overrun,
// append a NUL terminator here if there isn't already one.
if (m_secure_buffer.is_empty() || (m_secure_buffer[m_secure_buffer.size() - 1] != 0)) {
u8 nul = '\0';
m_secure_buffer.append(&nul, 1);
}
}
SecretString::~SecretString()