[PATCH 0/1] MR1637: server: Fix buffer overrun in map_view handler.

[Alex Henrie (＠alexhenrie)]

Because of padding at the end of the struct, sizeof(*view) is greater
than offsetof(struct memory_view, name[0]). Change the allocation to
overallocate slightly instead of underallocating slightly.

[PATCH 1/1] server: Fix buffer overrun in map_view handler.

[Alex Henrie]

From: Alex Henrie

Because of padding at the end of the struct, sizeof(*view) is greater
than offsetof(struct memory_view, name[0]). Change the allocation to
overallocate slightly instead of underallocating slightly.
---
 server/mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/mapping.c b/server/mapping.c
index 8d4332d240f..ed81a718bbe 100644
--- a/server/mapping.c
+++ b/server/mapping.c
@@ -1212,7 +1212,7 @@ DECL_HANDLER(map_view)
     if (!req->mapping)  /* image mapping for a .so dll */
     {
         if (get_req_data_size() > sizeof(view->image)) namelen = get_req_data_size() - sizeof(view->image);
-        if (!(view = mem_alloc( offsetof( struct memory_view, name[namelen] )))) return;
+        if (!(view = mem_alloc( sizeof(struct memory_view) + namelen * sizeof(WCHAR) ))) return;
         memset( view, 0, sizeof(*view) );
         view->base    = req->base;
         view->size    = req->size;

[Rémi Bernon (＠rbernon)]

What about using flexible array member? We've fixed similar issues elsewhere that way. Adding a `C_ASSERT( sizeof(struct memory_view) == offsetof(struct memory_view, name[0]) );` next to the struct definition also makes sure it is correctly alignmed.

[Alex Henrie (＠alexhenrie)]

I didn't know about C_ASSERT, thanks for pointing it out to me! I tried changing name[1] to name[] or name[0], but either way the C_ASSERT still failed because MinGW still added four bytes of padding at the end.