AK: Add trivial structure validation to SharedBuffer

If we're sharing buffers, we only want to share trivial structures
as anything else could potentially share internal pointers, which
most likely is going to cause problems due to different address
spaces.

Fix the GUI::SystemTheme structure, which was not trivial, which
is now caught at compile time.

Fixes #3650

16 files changed, 48 insertions, 29 deletions

diff --git a/AK/SharedBuffer.h b/AK/SharedBuffer.h
index 2a46a5a803..06e413eeae 100644
--- a/AK/SharedBuffer.h
+++ b/AK/SharedBuffer.h
@@ -44,8 +44,21 @@ public:
     int shbuf_id() const { return m_shbuf_id; }
     void seal();
     int size() const { return m_size; }
-    void* data() { return m_data; }
-    const void* data() const { return m_data; }
+
+    template<typename T>
+    T* data()
+    {
+        static_assert(IsVoid<T>::value || is_trivial<T>());
+        return (T*)m_data;
+    }
+
+    template<typename T>
+    const T* data() const
+    {
+        static_assert(IsVoid<T>::value || is_trivial<T>());
+        return (const T*)m_data;
+    }
+
     void set_volatile();
     [[nodiscard]] bool set_nonvolatile();
 
diff --git a/Libraries/LibAudio/Buffer.h b/Libraries/LibAudio/Buffer.h
index 3fcec5dd9f..24001176c3 100644
--- a/Libraries/LibAudio/Buffer.h
+++ b/Libraries/LibAudio/Buffer.h
@@ -120,7 +120,7 @@ public:
 
     const Sample* samples() const { return (const Sample*)data(); }
     int sample_count() const { return m_sample_count; }
-    const void* data() const { return m_buffer->data(); }
+    const void* data() const { return m_buffer->data<void>(); }
     int size_in_bytes() const { return m_sample_count * (int)sizeof(Sample); }
     int shbuf_id() const { return m_buffer->shbuf_id(); }
     SharedBuffer& shared_buffer() { return *m_buffer; }
@@ -130,7 +130,7 @@ private:
         : m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(Sample)))
         , m_sample_count(samples.size())
     {
-        memcpy(m_buffer->data(), samples.data(), samples.size() * sizeof(Sample));
+        memcpy(m_buffer->data<void>(), samples.data(), samples.size() * sizeof(Sample));
     }
 
     explicit Buffer(NonnullRefPtr<SharedBuffer>&& buffer, int sample_count)
diff --git a/Libraries/LibGUI/Clipboard.cpp b/Libraries/LibGUI/Clipboard.cpp
index 33c64c7157..ae023f0f9e 100644
--- a/Libraries/LibGUI/Clipboard.cpp
+++ b/Libraries/LibGUI/Clipboard.cpp
@@ -90,7 +90,7 @@ Clipboard::DataAndType Clipboard::data_and_type() const
         dbgprintf("GUI::Clipboard::data() clipping contents size is greater than shared buffer size\n");
         return {};
     }
-    auto data = ByteBuffer::copy(shared_buffer->data(), response->data_size());
+    auto data = ByteBuffer::copy(shared_buffer->data<void>(), response->data_size());
     auto type = response->mime_type();
     auto metadata = response->metadata().entries();
     return { data, type, metadata };
@@ -104,7 +104,7 @@ void Clipboard::set_data(ReadonlyBytes data, const String& type, const HashMap<S
         return;
     }
     if (!data.is_empty())
-        memcpy(shared_buffer->data(), data.data(), data.size());
+        memcpy(shared_buffer->data<void>(), data.data(), data.size());
     shared_buffer->seal();
     shared_buffer->share_with(connection().server_pid());
 
diff --git a/Libraries/LibGUI/Menu.cpp b/Libraries/LibGUI/Menu.cpp
index 71c162edf2..445a95394f 100644
--- a/Libraries/LibGUI/Menu.cpp
+++ b/Libraries/LibGUI/Menu.cpp
@@ -118,7 +118,7 @@ static int ensure_realized_icon(IconContainerType& container)
             auto shared_buffer = SharedBuffer::create_with_size(container.icon()->size_in_bytes());
             ASSERT(shared_buffer);
             auto shared_icon = Gfx::Bitmap::create_with_shared_buffer(Gfx::BitmapFormat::RGBA32, *shared_buffer, container.icon()->size());
-            memcpy(shared_buffer->data(), container.icon()->scanline_u8(0), container.icon()->size_in_bytes());
+            memcpy(shared_buffer->template data<u8>(), container.icon()->scanline_u8(0), container.icon()->size_in_bytes());
             shared_buffer->seal();
             shared_buffer->share_with(WindowServerConnection::the().server_pid());
             container.set_icon(shared_icon);
diff --git a/Libraries/LibGfx/Bitmap.cpp b/Libraries/LibGfx/Bitmap.cpp
index 581397dbb7..21f7a67c0e 100644
--- a/Libraries/LibGfx/Bitmap.cpp
+++ b/Libraries/LibGfx/Bitmap.cpp
@@ -169,7 +169,7 @@ RefPtr<Bitmap> Bitmap::create_with_shared_buffer(BitmapFormat format, NonnullRef
 
 Bitmap::Bitmap(BitmapFormat format, NonnullRefPtr<SharedBuffer>&& shared_buffer, const IntSize& size, const Vector<RGBA32>& palette)
     : m_size(size)
-    , m_data(shared_buffer->data())
+    , m_data(shared_buffer->data<void>())
     , m_pitch(minimum_pitch(size.width(), format))
     , m_format(format)
     , m_shared_buffer(move(shared_buffer))
@@ -255,7 +255,7 @@ RefPtr<Bitmap> Bitmap::to_bitmap_backed_by_shared_buffer() const
     auto bitmap = Bitmap::create_with_shared_buffer(m_format, *buffer, m_size, palette_to_vector());
     if (!bitmap)
         return nullptr;
-    memcpy(buffer->data(), scanline(0), size_in_bytes());
+    memcpy(buffer->data<void>(), scanline(0), size_in_bytes());
     return bitmap;
 }
 
diff --git a/Libraries/LibGfx/Palette.cpp b/Libraries/LibGfx/Palette.cpp
index 84d1990344..de6915d7ba 100644
--- a/Libraries/LibGfx/Palette.cpp
+++ b/Libraries/LibGfx/Palette.cpp
@@ -52,13 +52,13 @@ Palette::~Palette()
 
 const SystemTheme& PaletteImpl::theme() const
 {
-    return *(const SystemTheme*)m_theme_buffer->data();
+    return *m_theme_buffer->data<SystemTheme>();
 }
 
 Color PaletteImpl::color(ColorRole role) const
 {
     ASSERT((int)role < (int)ColorRole::__Count);
-    return theme().color[(int)role];
+    return Color::from_rgba(theme().color[(int)role]);
 }
 
 int PaletteImpl::metric(MetricRole role) const
@@ -76,7 +76,7 @@ String PaletteImpl::path(PathRole role) const
 NonnullRefPtr<PaletteImpl> PaletteImpl::clone() const
 {
     auto new_theme_buffer = SharedBuffer::create_with_size(m_theme_buffer->size());
-    memcpy(new_theme_buffer->data(), m_theme_buffer->data(), m_theme_buffer->size());
+    memcpy(new_theme_buffer->data<SystemTheme>(), &theme(), m_theme_buffer->size());
     return adopt(*new PaletteImpl(*new_theme_buffer));
 }
 
@@ -85,7 +85,7 @@ void Palette::set_color(ColorRole role, Color color)
     if (m_impl->ref_count() != 1)
         m_impl = m_impl->clone();
     auto& theme = const_cast<SystemTheme&>(impl().theme());
-    theme.color[(int)role] = color;
+    theme.color[(int)role] = color.value();
 }
 
 void Palette::set_metric(MetricRole role, int value)
@@ -101,7 +101,8 @@ void Palette::set_path(PathRole role, String path)
     if (m_impl->ref_count() != 1)
         m_impl = m_impl->clone();
     auto& theme = const_cast<SystemTheme&>(impl().theme());
-    theme.path[(int)role] = path;
+    memcpy(theme.path[(int)role], path.characters(), min(path.length() + 1, sizeof(theme.path[(int)role])));
+    theme.path[(int)role][sizeof(theme.path[(int)role]) - 1] = '\0';
 }
 
 PaletteImpl::~PaletteImpl()
diff --git a/Libraries/LibGfx/SystemTheme.cpp b/Libraries/LibGfx/SystemTheme.cpp
index 92b4f6cf12..ab9527b58b 100644
--- a/Libraries/LibGfx/SystemTheme.cpp
+++ b/Libraries/LibGfx/SystemTheme.cpp
@@ -27,6 +27,7 @@
 #include <AK/SharedBuffer.h>
 #include <LibCore/ConfigFile.h>
 #include <LibGfx/SystemTheme.h>
+#include <string.h>
 
 namespace Gfx {
 
@@ -49,7 +50,7 @@ int current_system_theme_buffer_id()
 void set_system_theme(SharedBuffer& buffer)
 {
     theme_buffer = buffer;
-    theme_page = (SystemTheme*)theme_buffer->data();
+    theme_page = theme_buffer->data<SystemTheme>();
 }
 
 RefPtr<SharedBuffer> load_system_theme(const String& path)
@@ -57,7 +58,7 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
     auto file = Core::ConfigFile::open(path);
     auto buffer = SharedBuffer::create_with_size(sizeof(SystemTheme));
 
-    auto* data = (SystemTheme*)buffer->data();
+    auto* data = buffer->data<SystemTheme>();
 
     auto get_color = [&](auto& name) {
         auto color_string = file->read_entry("Colors", name);
@@ -100,7 +101,7 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
 
 #undef __ENUMERATE_COLOR_ROLE
 #define __ENUMERATE_COLOR_ROLE(role) \
-    data->color[(int)ColorRole::role] = get_color(#role);
+    data->color[(int)ColorRole::role] = get_color(#role).value();
     ENUMERATE_COLOR_ROLES(__ENUMERATE_COLOR_ROLE)
 #undef __ENUMERATE_COLOR_ROLE
 
@@ -111,8 +112,12 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
     DO_METRIC(TitleButtonWidth);
     DO_METRIC(TitleButtonHeight);
 
-#define DO_PATH(x) \
-    data->path[(int)PathRole::x] = get_path(#x, (int)PathRole::x)
+#define DO_PATH(x)                                                                                               \
+    do {                                                                                                         \
+        auto path = get_path(#x, (int)PathRole::x);                                                              \
+        memcpy(data->path[(int)PathRole::x], path, min(strlen(path) + 1, sizeof(data->path[(int)PathRole::x]))); \
+        data->path[(int)PathRole::x][sizeof(data->path[(int)PathRole::x]) - 1] = '\0';                           \
+    } while (0)
 
     DO_PATH(TitleButtonIcons);
 
diff --git a/Libraries/LibGfx/SystemTheme.h b/Libraries/LibGfx/SystemTheme.h
index 5af10896a9..a81cba7d7b 100644
--- a/Libraries/LibGfx/SystemTheme.h
+++ b/Libraries/LibGfx/SystemTheme.h
@@ -143,9 +143,9 @@ enum class PathRole {
 };
 
 struct SystemTheme {
-    Color color[(int)ColorRole::__Count];
+    RGBA32 color[(int)ColorRole::__Count];
     int metric[(int)MetricRole::__Count];
-    String path[(int)PathRole::__Count];
+    char path[(int)PathRole::__Count][256]; // TODO: PATH_MAX?
 };
 
 const SystemTheme& current_system_theme();
diff --git a/Libraries/LibImageDecoderClient/Client.cpp b/Libraries/LibImageDecoderClient/Client.cpp
index 9aa7b2a4cd..04fc6135db 100644
--- a/Libraries/LibImageDecoderClient/Client.cpp
+++ b/Libraries/LibImageDecoderClient/Client.cpp
@@ -57,7 +57,7 @@ RefPtr<Gfx::Bitmap> Client::decode_image(const ByteBuffer& encoded_data)
         return nullptr;
     }
 
-    memcpy(encoded_buffer->data(), encoded_data.data(), encoded_data.size());
+    memcpy(encoded_buffer->data<void>(), encoded_data.data(), encoded_data.size());
 
     encoded_buffer->seal();
     encoded_buffer->share_with(server_pid());
diff --git a/Libraries/LibProtocol/Download.cpp b/Libraries/LibProtocol/Download.cpp
index 35c22fee04..3ed284ff9b 100644
--- a/Libraries/LibProtocol/Download.cpp
+++ b/Libraries/LibProtocol/Download.cpp
@@ -50,7 +50,7 @@ void Download::did_finish(Badge<Client>, bool success, Optional<u32> status_code
     RefPtr<SharedBuffer> shared_buffer;
     if (success && shbuf_id != -1) {
         shared_buffer = SharedBuffer::create_from_shbuf_id(shbuf_id);
-        payload = ByteBuffer::wrap(shared_buffer->data(), total_size);
+        payload = ByteBuffer::wrap(shared_buffer->data<void>(), total_size);
     }
 
     // FIXME: It's a bit silly that we copy the response headers here just so we can move them into a HashMap with different traits.
diff --git a/Services/Clipboard/ClientConnection.cpp b/Services/Clipboard/ClientConnection.cpp
index 209a5a699d..857dee4d64 100644
--- a/Services/Clipboard/ClientConnection.cpp
+++ b/Services/Clipboard/ClientConnection.cpp
@@ -83,7 +83,7 @@ OwnPtr<Messages::ClipboardServer::GetClipboardDataResponse> ClientConnection::ha
         //        It would be even nicer if a SharedBuffer could have an arbitrary number of clients..
         RefPtr<SharedBuffer> shared_buffer = SharedBuffer::create_with_size(storage.data_size());
         ASSERT(shared_buffer);
-        memcpy(shared_buffer->data(), storage.data(), storage.data_size());
+        memcpy(shared_buffer->data<void>(), storage.data(), storage.data_size());
         shared_buffer->seal();
         shared_buffer->share_with(client_pid());
         shbuf_id = shared_buffer->shbuf_id();
diff --git a/Services/Clipboard/Storage.cpp b/Services/Clipboard/Storage.cpp
index e1788dbf39..3b745dea16 100644
--- a/Services/Clipboard/Storage.cpp
+++ b/Services/Clipboard/Storage.cpp
@@ -46,7 +46,7 @@ Storage::~Storage()
 
 void Storage::set_data(NonnullRefPtr<SharedBuffer> data, size_t data_size, const String& mime_type, const HashMap<String, String>& metadata)
 {
-    dbg() << "Storage::set_data <- [" << mime_type << "] " << data->data() << " (" << data_size << " bytes)";
+    dbg() << "Storage::set_data <- [" << mime_type << "] " << data->data<void>() << " (" << data_size << " bytes)";
     for (auto& it : metadata) {
         dbg() << "  " << it.key << ": " << it.value;
     }
diff --git a/Services/Clipboard/Storage.h b/Services/Clipboard/Storage.h
index 956377fa25..ce194c15f4 100644
--- a/Services/Clipboard/Storage.h
+++ b/Services/Clipboard/Storage.h
@@ -47,7 +47,7 @@ public:
     {
         if (!has_data())
             return nullptr;
-        return static_cast<const u8*>(m_shared_buffer->data());
+        return m_shared_buffer->data<u8>();
     }
 
     size_t data_size() const
diff --git a/Services/ImageDecoder/ClientConnection.cpp b/Services/ImageDecoder/ClientConnection.cpp
index 42df7eb0bc..0a64b399b8 100644
--- a/Services/ImageDecoder/ClientConnection.cpp
+++ b/Services/ImageDecoder/ClientConnection.cpp
@@ -79,7 +79,7 @@ OwnPtr<Messages::ImageDecoderServer::DecodeImageResponse> ClientConnection::hand
     dbg() << "Trying to decode " << message.encoded_size() << " bytes of image(?) data in shbuf_id=" << message.encoded_shbuf_id() << " (shbuf size: " << encoded_buffer->size() << ")";
 #endif
 
-    auto decoder = Gfx::ImageDecoder::create((const u8*)encoded_buffer->data(), message.encoded_size());
+    auto decoder = Gfx::ImageDecoder::create(encoded_buffer->data<u8>(), message.encoded_size());
     auto bitmap = decoder->bitmap();
 
     if (!bitmap) {
diff --git a/Services/ProtocolServer/ClientConnection.cpp b/Services/ProtocolServer/ClientConnection.cpp
index 86cc6344ce..93f02ae2e6 100644
--- a/Services/ProtocolServer/ClientConnection.cpp
+++ b/Services/ProtocolServer/ClientConnection.cpp
@@ -91,7 +91,7 @@ void ClientConnection::did_finish_download(Badge<Download>, Download& download,
     RefPtr<SharedBuffer> buffer;
     if (success && download.payload().size() > 0 && !download.payload().is_null()) {
         buffer = SharedBuffer::create_with_size(download.payload().size());
-        memcpy(buffer->data(), download.payload().data(), download.payload().size());
+        memcpy(buffer->data<void>(), download.payload().data(), download.payload().size());
         buffer->seal();
         buffer->share_with(client_pid());
         m_shared_buffers.set(buffer->shbuf_id(), buffer);
diff --git a/Services/WebContent/PageHost.cpp b/Services/WebContent/PageHost.cpp
index 90c763d16a..f6603645ad 100644
--- a/Services/WebContent/PageHost.cpp
+++ b/Services/WebContent/PageHost.cpp
@@ -50,7 +50,7 @@ void PageHost::setup_palette()
 {
     // FIXME: Get the proper palette from our peer somehow
     auto buffer = SharedBuffer::create_with_size(sizeof(Gfx::SystemTheme));
-    auto* theme = (Gfx::SystemTheme*)buffer->data();
+    auto* theme = buffer->data<Gfx::SystemTheme>();
     theme->color[(int)Gfx::ColorRole::Window] = Color::Magenta;
     theme->color[(int)Gfx::ColorRole::WindowText] = Color::Cyan;
     m_palette_impl = Gfx::PaletteImpl::create_with_shared_buffer(*buffer);