From 38b5ff0cec266c7c5e423b68e8626f4a4a63d3ee Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sun, 16 Nov 2025 03:34:05 +0900 Subject: [PATCH] internal/wayland: check pathname size This avoids passing a truncated pathname to the kernel. Signed-off-by: Ophestra --- internal/wayland/conn_test.go | 54 ++++++++++++++ internal/wayland/wayland-client-helper.c | 1 - internal/wayland/wayland-client-helper.h | 9 +++ internal/wayland/wayland.go | 9 ++- internal/wayland/wayland_test.go | 95 +++++++++++++++--------- 5 files changed, 129 insertions(+), 39 deletions(-) create mode 100644 internal/wayland/conn_test.go diff --git a/internal/wayland/conn_test.go b/internal/wayland/conn_test.go new file mode 100644 index 0000000..4be6dbf --- /dev/null +++ b/internal/wayland/conn_test.go @@ -0,0 +1,54 @@ +package wayland + +import ( + "errors" + "os" + "reflect" + "syscall" + "testing" + + "hakurei.app/container/check" +) + +func TestSecurityContextClose(t *testing.T) { + t.Parallel() + + if err := (*SecurityContext)(nil).Close(); !reflect.DeepEqual(err, os.ErrInvalid) { + t.Fatalf("Close: error = %v", err) + } + + var ctx SecurityContext + if err := syscall.Pipe2(ctx.closeFds[0:], syscall.O_CLOEXEC); err != nil { + t.Fatalf("Pipe: error = %v", err) + } + t.Cleanup(func() { _ = syscall.Close(ctx.closeFds[0]); _ = syscall.Close(ctx.closeFds[1]) }) + + if err := ctx.Close(); err != nil { + t.Fatalf("Close: error = %v", err) + } + + wantErr := errors.Join(syscall.EBADF, syscall.EBADF) + if err := ctx.Close(); !reflect.DeepEqual(err, wantErr) { + t.Fatalf("Close: error = %#v, want %#v", err, wantErr) + } +} + +func TestNewEnsure(t *testing.T) { + existingDirPath := check.MustAbs(t.TempDir()).Append("dir") + if err := os.MkdirAll(existingDirPath.String(), 0700); err != nil { + t.Fatal(err) + } + nonexistent := check.MustAbs("/proc/nonexistent") + + wantErr := &Error{RCreate, existingDirPath.String(), nonexistent.String(), &os.PathError{ + Op: "open", + Path: existingDirPath.String(), + Err: syscall.EISDIR, + }} + if _, err := New( + nonexistent, + existingDirPath, "", "", + ); !reflect.DeepEqual(err, wantErr) { + t.Fatalf("New: error = %#v, want %#v", err, wantErr) + } +} diff --git a/internal/wayland/wayland-client-helper.c b/internal/wayland/wayland-client-helper.c index be9ddf0..0fa0372 100644 --- a/internal/wayland/wayland-client-helper.c +++ b/internal/wayland/wayland-client-helper.c @@ -3,7 +3,6 @@ #include #include #include -#include #include #include "security-context-v1-protocol.h" diff --git a/internal/wayland/wayland-client-helper.h b/internal/wayland/wayland-client-helper.h index 5cdf033..0a2cffb 100644 --- a/internal/wayland/wayland-client-helper.h +++ b/internal/wayland/wayland-client-helper.h @@ -1,3 +1,6 @@ +#include +#include + typedef enum { HAKUREI_WAYLAND_SUCCESS, /* wl_display_connect_to_fd failed, errno */ @@ -29,3 +32,9 @@ hakurei_wayland_res hakurei_security_context_bind( const char *app_id, const char *instance_id, int close_fd); + +/* returns whether the specified size fits in the sun_path field of sockaddr_un */ +static inline bool hakurei_is_valid_size_sun_path(size_t sz) { + struct sockaddr_un sockaddr; + return sz <= sizeof(sockaddr.sun_path); +}; diff --git a/internal/wayland/wayland.go b/internal/wayland/wayland.go index f511300..cc4f4e0 100644 --- a/internal/wayland/wayland.go +++ b/internal/wayland/wayland.go @@ -12,8 +12,8 @@ package wayland */ import "C" import ( + "errors" "strings" - "syscall" ) const ( @@ -134,8 +134,11 @@ func securityContextBind( appID, instanceID string, closeFd int, ) error { - if hasNull(appID) || hasNull(instanceID) { - return syscall.EINVAL + if hasNull(socketPath) || hasNull(appID) || hasNull(instanceID) { + return &Error{Cause: RBind, Path: socketPath, Errno: errors.New("argument contains NUL character")} + } + if !C.hakurei_is_valid_size_sun_path(C.size_t(len(socketPath))) { + return &Error{Cause: RBind, Path: socketPath, Errno: errors.New("socket pathname too long")} } var e Error diff --git a/internal/wayland/wayland_test.go b/internal/wayland/wayland_test.go index 16ab22a..c5e6c55 100644 --- a/internal/wayland/wayland_test.go +++ b/internal/wayland/wayland_test.go @@ -1,12 +1,13 @@ -package wayland_test +package wayland import ( + "errors" "os" + "reflect" "syscall" "testing" "hakurei.app/container/stub" - "hakurei.app/internal/wayland" ) func TestError(t *testing.T) { @@ -14,88 +15,88 @@ func TestError(t *testing.T) { testCases := []struct { name string - err wayland.Error + err Error want string }{ - {"success", wayland.Error{ - Cause: wayland.RSuccess, + {"success", Error{ + Cause: RSuccess, }, "success"}, - {"success errno", wayland.Error{ - Cause: wayland.RSuccess, + {"success errno", Error{ + Cause: RSuccess, Errno: stub.UniqueError(0), }, "unique error 0 injected by the test suite"}, - {"wl_display_connect_to_fd", wayland.Error{ - Cause: wayland.RConnect, + {"wl_display_connect_to_fd", Error{ + Cause: RConnect, Errno: stub.UniqueError(1), }, "wl_display_connect_to_fd failed: unique error 1 injected by the test suite"}, - {"wl_registry_add_listener", wayland.Error{ - Cause: wayland.RListener, + {"wl_registry_add_listener", Error{ + Cause: RListener, Errno: stub.UniqueError(2), }, "wl_registry_add_listener failed: unique error 2 injected by the test suite"}, - {"wl_display_roundtrip", wayland.Error{ - Cause: wayland.RRoundtrip, + {"wl_display_roundtrip", Error{ + Cause: RRoundtrip, Errno: stub.UniqueError(3), }, "wl_display_roundtrip failed: unique error 3 injected by the test suite"}, - {"not available", wayland.Error{ - Cause: wayland.RNotAvail, + {"not available", Error{ + Cause: RNotAvail, }, "compositor does not implement security_context_v1"}, - {"not available errno", wayland.Error{ - Cause: wayland.RNotAvail, + {"not available errno", Error{ + Cause: RNotAvail, Errno: syscall.EAGAIN, }, "compositor does not implement security_context_v1"}, - {"socket", wayland.Error{ - Cause: wayland.RSocket, + {"socket", Error{ + Cause: RSocket, Errno: stub.UniqueError(4), }, "socket: unique error 4 injected by the test suite"}, - {"bind", wayland.Error{ - Cause: wayland.RBind, + {"bind", Error{ + Cause: RBind, Path: "/hakurei.0/18783d07791f2460dbbcffb76c24c9e6/wayland", Errno: stub.UniqueError(5), }, "cannot bind /hakurei.0/18783d07791f2460dbbcffb76c24c9e6/wayland: unique error 5 injected by the test suite"}, - {"listen", wayland.Error{ - Cause: wayland.RListen, + {"listen", Error{ + Cause: RListen, Path: "/hakurei.0/18783d07791f2460dbbcffb76c24c9e6/wayland", Errno: stub.UniqueError(6), }, "cannot listen on /hakurei.0/18783d07791f2460dbbcffb76c24c9e6/wayland: unique error 6 injected by the test suite"}, - {"socket invalid", wayland.Error{ - Cause: wayland.RSocket, + {"socket invalid", Error{ + Cause: RSocket, }, "socket operation failed"}, - {"create", wayland.Error{ - Cause: wayland.RCreate, + {"create", Error{ + Cause: RCreate, }, "cannot ensure wayland pathname socket"}, - {"create path", wayland.Error{ - Cause: wayland.RCreate, + {"create path", Error{ + Cause: RCreate, Errno: &os.PathError{Op: "create", Path: "/proc/nonexistent", Err: syscall.EEXIST}, }, "create /proc/nonexistent: file exists"}, - {"host socket", wayland.Error{ - Cause: wayland.RHostSocket, + {"host socket", Error{ + Cause: RHostSocket, Errno: stub.UniqueError(7), }, "socket: unique error 7 injected by the test suite"}, - {"host connect", wayland.Error{ - Cause: wayland.RHostConnect, + {"host connect", Error{ + Cause: RHostConnect, Host: "/run/user/1971/wayland-1", Errno: stub.UniqueError(8), }, "cannot connect to /run/user/1971/wayland-1: unique error 8 injected by the test suite"}, - {"invalid", wayland.Error{ + {"invalid", Error{ Cause: 0xbad, }, "impossible outcome"}, - {"invalid errno", wayland.Error{ + {"invalid errno", Error{ Cause: 0xbad, Errno: stub.UniqueError(9), }, "impossible outcome: unique error 9 injected by the test suite"}, @@ -110,3 +111,27 @@ func TestError(t *testing.T) { }) } } + +func TestSecurityContextBindValidate(t *testing.T) { + t.Parallel() + + t.Run("NUL", func(t *testing.T) { + t.Parallel() + + want := &Error{Cause: RBind, Path: "\x00", Errno: errors.New("argument contains NUL character")} + if got := securityContextBind("\x00", -1, "\x00", "\x00", -1); !reflect.DeepEqual(got, want) { + t.Fatalf("securityContextBind: error = %#v, want %#v", got, want) + } + }) + + t.Run("long", func(t *testing.T) { + t.Parallel() + // 256 bytes + const oversizedPath = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + want := &Error{Cause: RBind, Path: oversizedPath, Errno: errors.New("socket pathname too long")} + if got := securityContextBind(oversizedPath, -1, "", "", -1); !reflect.DeepEqual(got, want) { + t.Fatalf("securityContextBind: error = %#v, want %#v", got, want) + } + }) +}