From 3bf69027904a8a6a32c7acf6684c8e2c4f7bdefb Mon Sep 17 00:00:00 2001
From: Brian Gianforcaro <bgianf@serenityos.org>
Date: Sun, 12 Sep 2021 06:54:57 -0700
Subject: [PATCH] LibCore: Add SecretString, a buffer that is zero'd on
 destruction

We have a few places where we read secrets into memory, and then
do some computation on them. In these cases we should always make
sure we zero the allocations before they are free'd.

The SecureString wrapper provides this abstraction by wrapping a
ByteBuffer and calling explicit_bzero on destruction of the object.
---
 Userland/Libraries/LibCore/CMakeLists.txt   |  1 +
 Userland/Libraries/LibCore/SecretString.cpp | 42 +++++++++++++++++++++
 Userland/Libraries/LibCore/SecretString.h   | 38 +++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 Userland/Libraries/LibCore/SecretString.cpp
 create mode 100644 Userland/Libraries/LibCore/SecretString.h

diff --git a/Userland/Libraries/LibCore/CMakeLists.txt b/Userland/Libraries/LibCore/CMakeLists.txt
index 311168677b..a87e8fb8a9 100644
--- a/Userland/Libraries/LibCore/CMakeLists.txt
+++ b/Userland/Libraries/LibCore/CMakeLists.txt
@@ -24,6 +24,7 @@ set(SOURCES
     Process.cpp
     ProcessStatisticsReader.cpp
     Property.cpp
+    SecretString.cpp
     Socket.cpp
     StandardPaths.cpp
     TCPServer.cpp
diff --git a/Userland/Libraries/LibCore/SecretString.cpp b/Userland/Libraries/LibCore/SecretString.cpp
new file mode 100644
index 0000000000..398aa96300
--- /dev/null
+++ b/Userland/Libraries/LibCore/SecretString.cpp
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2021, Brian Gianforcaro <bgianf@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibCore/SecretString.h>
+#include <string.h>
+
+namespace Core {
+
+SecretString SecretString::take_ownership(char*& cstring, size_t length)
+{
+    auto buffer = ByteBuffer::copy(cstring, length);
+    VERIFY(buffer.has_value());
+
+    explicit_bzero(cstring, length);
+    free(cstring);
+
+    return SecretString(buffer.release_value());
+}
+
+SecretString SecretString::take_ownership(ByteBuffer&& buffer)
+{
+    return SecretString(move(buffer));
+}
+
+SecretString::SecretString(ByteBuffer&& buffer)
+    : m_secure_buffer(move(buffer))
+{
+}
+
+SecretString::~SecretString()
+{
+    if (!m_secure_buffer.is_empty()) {
+        // Note: We use explicit_bzero to avoid the zeroing from being optimized out by the compiler,
+        // which is possible if memset was to be used here.
+        explicit_bzero(m_secure_buffer.data(), m_secure_buffer.capacity());
+    }
+}
+
+}
diff --git a/Userland/Libraries/LibCore/SecretString.h b/Userland/Libraries/LibCore/SecretString.h
new file mode 100644
index 0000000000..865542ff7c
--- /dev/null
+++ b/Userland/Libraries/LibCore/SecretString.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2021, Brian Gianforcaro <bgianf@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/ByteBuffer.h>
+#include <AK/Noncopyable.h>
+#include <AK/StringView.h>
+
+namespace Core {
+
+class SecretString {
+    AK_MAKE_NONCOPYABLE(SecretString);
+
+public:
+    [[nodiscard]] static SecretString take_ownership(char*&, size_t);
+    [[nodiscard]] static SecretString take_ownership(ByteBuffer&&);
+
+    [[nodiscard]] bool is_empty() const { return m_secure_buffer.is_empty(); }
+    [[nodiscard]] size_t length() const { return m_secure_buffer.size(); }
+    [[nodiscard]] char const* characters() const { return reinterpret_cast<const char*>(m_secure_buffer.data()); }
+    [[nodiscard]] StringView view() const { return { characters(), length() }; }
+
+    SecretString() = default;
+    ~SecretString();
+    SecretString(SecretString&&) = default;
+    SecretString& operator=(SecretString&&) = default;
+
+private:
+    explicit SecretString(ByteBuffer&&);
+
+    ByteBuffer m_secure_buffer;
+};
+
+}