1
0
mirror of https://github.com/AvengeMedia/DankMaterialShell.git synced 2026-08-05 21:18:30 -04:00

core: fix security and concurrency issues found in a backend audit (#2805)

* core: fix security and concurrency issues found in backend audit

Security:
- privesc: pipe the sudo password via stdin (sudo -S) instead of
  embedding it in the command string, so it no longer appears in argv
  (readable by any local user via /proc/<pid>/cmdline or ps)
- greeter: tokenize a session .desktop Exec= line into argv and execve
  directly instead of running it through /bin/sh -c, closing a command-
  injection path via user-writable ~/.local/share/wayland-sessions
- plugins: reject path-separator/.. in plugin id/name before joining
  into a filesystem path, closing an arbitrary-directory-delete in the
  uninstall/update fallback
- keybinds/hyprland: always quote unrecognized bind actions/keys when
  writing generated Lua; only re-emit genuine round-tripped custom Lua
  verbatim (tracked via an explicit flag), closing a Lua-injection path
- desktop/mimeapps: reject newline/bracket in mime/desktop-id fields so
  they can't inject fake sections into the shared mimeapps.list

Robustness / concurrency:
- server: recover panics in the request-dispatch path so one bad
  handler can't crash the daemon and drop every client
- go-wayland: recover panics in the shared dispatch choke point so a
  malformed compositor event can't crash CLI tools / the daemon
- server: per-connection D-Bus client ID instead of a shared constant,
  fixing cross-client signal delivery and subscription teardown
- network: guard the NetworkManager device maps with a mutex (a
  concurrent map read/write here is an unrecoverable fatal error)
- cups: close the event channel on Stop() so Unsubscribe() of the last
  subscriber no longer deadlocks; allocate the fresh channel in Start()
- freedesktop: reuse the shared session conn for the settings watcher
  and tear it down in Close(), fixing a per-Manager conn+goroutine leak
- clipboard: mutex-guard lazy dbusConn creation
- geolocation: use WithMatchMember for the GeoClue2 LocationUpdated
  signal (was WithMatchSender with an interface.member string, so the
  match never fired and live location updates never arrived)
- screenshot: set failed=true on buffer/pool creation errors so the
  dispatch loop doesn't wait forever for a ready/failed that never comes

* apply code review comments

---------

Co-authored-by: bbedward <bbedward@gmail.com>
(cherry picked from commit ca89e12963)
This commit is contained in:
Rafi
2026-07-13 15:44:42 -04:00
committed by bbedward
parent 7f37799e53
commit d96559f7df
25 changed files with 522 additions and 132 deletions
+24 -11
View File
@@ -1839,21 +1839,34 @@ func (m *Manager) EntryToFile(entry *Entry) string {
return ""
}
func (m *Manager) dbusConnForFlatpak() (*dbus.Conn, error) {
m.dbusConnMutex.Lock()
defer m.dbusConnMutex.Unlock()
if m.dbusConn != nil {
return m.dbusConn, nil
}
conn, err := dbus.ConnectSessionBus()
if err != nil {
return nil, fmt.Errorf("connect session bus: %w", err)
}
if !conn.SupportsUnixFDs() {
conn.Close()
return nil, fmt.Errorf("D-Bus connection does not support Unix FD passing")
}
m.dbusConn = conn
return conn, nil
}
func (m *Manager) ExportFileForFlatpak(filePath string) (string, error) {
if _, err := os.Stat(filePath); err != nil {
return "", fmt.Errorf("file not found: %w", err)
}
if m.dbusConn == nil {
conn, err := dbus.ConnectSessionBus()
if err != nil {
return "", fmt.Errorf("connect session bus: %w", err)
}
if !conn.SupportsUnixFDs() {
conn.Close()
return "", fmt.Errorf("D-Bus connection does not support Unix FD passing")
}
m.dbusConn = conn
dbusConn, err := m.dbusConnForFlatpak()
if err != nil {
return "", err
}
file, err := os.Open(filePath)
@@ -1862,7 +1875,7 @@ func (m *Manager) ExportFileForFlatpak(filePath string) (string, error) {
}
fd := int(file.Fd())
portal := m.dbusConn.Object("org.freedesktop.portal.Documents", "/org/freedesktop/portal/documents")
portal := dbusConn.Object("org.freedesktop.portal.Documents", "/org/freedesktop/portal/documents")
var docIds []string
var extra map[string]dbus.Variant
+3 -1
View File
@@ -153,7 +153,9 @@ type Manager struct {
notifierWg sync.WaitGroup
lastState *State
dbusConn *dbus.Conn
// lazily created by dbusConnForFlatpak under dbusConnMutex
dbusConn *dbus.Conn
dbusConnMutex sync.Mutex
}
func (m *Manager) GetState() State {
+12
View File
@@ -37,6 +37,9 @@ func (sm *SubscriptionManager) Start() error {
return fmt.Errorf("subscription manager already running")
}
sm.running = true
// replace the channel closed by the previous Stop(); doing it here rather
// than in Stop() guarantees a lagging eventHandler still observes the close
sm.eventChan = make(chan SubscriptionEvent, 100)
sm.mu.Unlock()
subID, err := sm.createSubscription()
@@ -206,6 +209,8 @@ func (sm *SubscriptionManager) parseEvent(attrs ipp.Attributes) SubscriptionEven
}
func (sm *SubscriptionManager) Events() <-chan SubscriptionEvent {
sm.mu.Lock()
defer sm.mu.Unlock()
return sm.eventChan
}
@@ -228,6 +233,13 @@ func (sm *SubscriptionManager) Stop() {
}
sm.stopChan = make(chan struct{})
// the writer (notificationLoop) joined above, so closing is safe; without
// this close Manager.eventHandler never returns and Unsubscribe deadlocks
// on eventWG.Wait(). Start() allocates the replacement.
sm.mu.Lock()
close(sm.eventChan)
sm.mu.Unlock()
}
func (sm *SubscriptionManager) cancelSubscription() {
@@ -38,6 +38,8 @@ func (sm *DBusSubscriptionManager) Start() error {
return fmt.Errorf("subscription manager already running")
}
sm.running = true
// replaced here rather than in Stop(); see SubscriptionManager.Start()
sm.eventChan = make(chan SubscriptionEvent, 100)
sm.mu.Unlock()
conn, err := dbus.ConnectSystemBus()
@@ -252,6 +254,8 @@ func (sm *DBusSubscriptionManager) parseDBusSignal(sig *dbus.Signal) Subscriptio
}
func (sm *DBusSubscriptionManager) Events() <-chan SubscriptionEvent {
sm.mu.Lock()
defer sm.mu.Unlock()
return sm.eventChan
}
@@ -278,6 +282,12 @@ func (sm *DBusSubscriptionManager) Stop() {
}
sm.stopChan = make(chan struct{})
// the writer (dbusListenerLoop) joined above, so closing is safe; see
// SubscriptionManager.Stop()
sm.mu.Lock()
close(sm.eventChan)
sm.mu.Unlock()
}
func (sm *DBusSubscriptionManager) cancelSubscription() {
+4
View File
@@ -201,6 +201,10 @@ func handleListNames(conn net.Conn, req models.Request, m *Manager) {
}
func handleSubscribe(conn net.Conn, req models.Request, m *Manager, clientID string) {
if id := params.StringOpt(req.Params, "clientId", ""); id != "" {
clientID = id
}
bus, err := params.String(req.Params, "bus")
if err != nil {
models.RespondError(conn, req.ID, err.Error())
+19 -4
View File
@@ -137,22 +137,25 @@ func (m *Manager) consumeSelfEcho(value uint32) bool {
}
func (m *Manager) watchSettingsChanges() {
conn, err := dbus.ConnectSessionBus()
if err != nil {
log.Warnf("color-scheme watcher: session bus connect: %v", err)
// reuse the shared session connection; a dedicated one was unreachable
// from Close() and leaked with this goroutine
if m.sessionConn == nil {
return
}
conn := m.sessionConn
if err := conn.AddMatchSignal(
dbus.WithMatchInterface(dbusPortalSettingsInterface),
dbus.WithMatchMember("SettingChanged"),
); err != nil {
log.Warnf("Failed to watch portal settings changes: %v", err)
conn.Close()
return
}
signals := make(chan *dbus.Signal, 64)
m.stateMutex.Lock()
m.settingsSignals = signals
m.stateMutex.Unlock()
conn.Signal(signals)
for sig := range signals {
@@ -309,6 +312,18 @@ func (m *Manager) Close() {
m.systemConn.Close()
}
if m.sessionConn != nil {
m.sessionConn.RemoveMatchSignal(
dbus.WithMatchInterface(dbusPortalSettingsInterface),
dbus.WithMatchMember("SettingChanged"),
)
m.stateMutex.Lock()
signals := m.settingsSignals
m.settingsSignals = nil
m.stateMutex.Unlock()
if signals != nil {
m.sessionConn.RemoveSignal(signals)
close(signals)
}
m.sessionConn.Close()
}
}
@@ -71,4 +71,6 @@ type Manager struct {
screensaverGnomeClaimed bool
selfEchoMu sync.Mutex
selfEchoes []colorSchemeEcho
// registered on sessionConn by watchSettingsChanges; guarded by stateMutex
settingsSignals chan *dbus.Signal
}
@@ -2,6 +2,7 @@ package network
import (
"fmt"
"maps"
"sync"
"github.com/AvengeMedia/DankMaterialShell/core/internal/log"
@@ -57,6 +58,11 @@ type NetworkManagerBackend struct {
wifiDev any
wifiDevices map[string]*wifiDeviceInfo
// devMutex guards ethernetDevices/wifiDevices (written by the signal pump,
// read by request handlers). Not reentrant — never hold it across calls
// into other backend methods.
devMutex sync.RWMutex
dbusConn *dbus.Conn
signals chan *dbus.Signal
sigWG sync.WaitGroup
@@ -185,12 +191,12 @@ func (b *NetworkManagerBackend) Initialize() error {
}
hwAddr, _ := w.GetPropertyHwAddress()
b.ethernetDevices[iface] = &ethernetDeviceInfo{
b.setEthernetDeviceInfo(iface, &ethernetDeviceInfo{
device: dev,
wired: w,
name: iface,
hwAddress: hwAddr,
}
})
if b.ethernetDevice == nil {
b.ethernetDevice = dev
@@ -214,12 +220,12 @@ func (b *NetworkManagerBackend) Initialize() error {
}
hwAddr, _ := w.GetPropertyHwAddress()
b.wifiDevices[iface] = &wifiDeviceInfo{
b.setWifiDeviceInfo(iface, &wifiDeviceInfo{
device: dev,
wireless: w,
name: iface,
hwAddress: hwAddr,
}
})
if b.wifiDevice == nil {
b.wifiDevice = dev
@@ -267,6 +273,80 @@ func (b *NetworkManagerBackend) Initialize() error {
return nil
}
func (b *NetworkManagerBackend) ethernetDevicesSnapshot() map[string]*ethernetDeviceInfo {
b.devMutex.RLock()
defer b.devMutex.RUnlock()
out := make(map[string]*ethernetDeviceInfo, len(b.ethernetDevices))
maps.Copy(out, b.ethernetDevices)
return out
}
func (b *NetworkManagerBackend) wifiDevicesSnapshot() map[string]*wifiDeviceInfo {
b.devMutex.RLock()
defer b.devMutex.RUnlock()
out := make(map[string]*wifiDeviceInfo, len(b.wifiDevices))
maps.Copy(out, b.wifiDevices)
return out
}
func (b *NetworkManagerBackend) ethernetDeviceByIface(iface string) (*ethernetDeviceInfo, bool) {
b.devMutex.RLock()
defer b.devMutex.RUnlock()
info, ok := b.ethernetDevices[iface]
return info, ok
}
func (b *NetworkManagerBackend) wifiDeviceByIface(iface string) (*wifiDeviceInfo, bool) {
b.devMutex.RLock()
defer b.devMutex.RUnlock()
info, ok := b.wifiDevices[iface]
return info, ok
}
func (b *NetworkManagerBackend) setEthernetDeviceInfo(iface string, info *ethernetDeviceInfo) {
b.devMutex.Lock()
b.ethernetDevices[iface] = info
b.devMutex.Unlock()
}
func (b *NetworkManagerBackend) setWifiDeviceInfo(iface string, info *wifiDeviceInfo) {
b.devMutex.Lock()
b.wifiDevices[iface] = info
b.devMutex.Unlock()
}
// removeEthernetDeviceByPath deletes the device and returns a snapshot of
// what's left so the caller can pick a replacement without holding devMutex
func (b *NetworkManagerBackend) removeEthernetDeviceByPath(path dbus.ObjectPath) (removed *ethernetDeviceInfo, remaining map[string]*ethernetDeviceInfo, found bool) {
b.devMutex.Lock()
defer b.devMutex.Unlock()
for iface, info := range b.ethernetDevices {
if info.device.GetPath() != path {
continue
}
delete(b.ethernetDevices, iface)
remaining = make(map[string]*ethernetDeviceInfo, len(b.ethernetDevices))
maps.Copy(remaining, b.ethernetDevices)
return info, remaining, true
}
return nil, nil, false
}
func (b *NetworkManagerBackend) removeWifiDeviceByPath(path dbus.ObjectPath) (removed *wifiDeviceInfo, remaining map[string]*wifiDeviceInfo, found bool) {
b.devMutex.Lock()
defer b.devMutex.Unlock()
for iface, info := range b.wifiDevices {
if info.device.GetPath() != path {
continue
}
delete(b.wifiDevices, iface)
remaining = make(map[string]*wifiDeviceInfo, len(b.wifiDevices))
maps.Copy(remaining, b.wifiDevices)
return info, remaining, true
}
return nil, nil, false
}
func (b *NetworkManagerBackend) Close() {
close(b.stopChan)
b.StopMonitoring()
@@ -323,7 +323,7 @@ func (b *NetworkManagerBackend) GetEthernetDevices() []EthernetDevice {
}
func (b *NetworkManagerBackend) DisconnectEthernetDevice(device string) error {
info, ok := b.ethernetDevices[device]
info, ok := b.ethernetDeviceByIface(device)
if !ok {
return fmt.Errorf("ethernet device %s not found", device)
}
@@ -345,9 +345,10 @@ func (b *NetworkManagerBackend) DisconnectEthernetDevice(device string) error {
}
func (b *NetworkManagerBackend) updateAllEthernetDevices() {
devices := make([]EthernetDevice, 0, len(b.ethernetDevices))
ethernetDevices := b.ethernetDevicesSnapshot()
devices := make([]EthernetDevice, 0, len(ethernetDevices))
for name, info := range b.ethernetDevices {
for name, info := range ethernetDevices {
state, _ := info.device.GetPropertyState()
connected := state == gonetworkmanager.NmDeviceStateActivated
driver, _ := info.device.GetPropertyDriver()
@@ -112,7 +112,7 @@ func (b *NetworkManagerBackend) startSignalPump() error {
return err
}
for _, info := range b.wifiDevices {
for _, info := range b.wifiDevicesSnapshot() {
if err := conn.AddMatchSignal(
dbus.WithMatchObjectPath(dbus.ObjectPath(info.device.GetPath())),
dbus.WithMatchInterface(dbusPropsInterface),
@@ -124,7 +124,7 @@ func (b *NetworkManagerBackend) startSignalPump() error {
}
}
for _, info := range b.ethernetDevices {
for _, info := range b.ethernetDevicesSnapshot() {
if err := conn.AddMatchSignal(
dbus.WithMatchObjectPath(dbus.ObjectPath(info.device.GetPath())),
dbus.WithMatchInterface(dbusPropsInterface),
@@ -227,7 +227,7 @@ func (b *NetworkManagerBackend) stopSignalPump() {
dbus.WithMatchMember("StateChanged"),
)
for _, info := range b.wifiDevices {
for _, info := range b.wifiDevicesSnapshot() {
b.dbusConn.RemoveMatchSignal(
dbus.WithMatchObjectPath(dbus.ObjectPath(info.device.GetPath())),
dbus.WithMatchInterface(dbusPropsInterface),
@@ -235,7 +235,7 @@ func (b *NetworkManagerBackend) stopSignalPump() {
)
}
for _, info := range b.ethernetDevices {
for _, info := range b.ethernetDevicesSnapshot() {
b.dbusConn.RemoveMatchSignal(
dbus.WithMatchObjectPath(dbus.ObjectPath(info.device.GetPath())),
dbus.WithMatchInterface(dbusPropsInterface),
@@ -550,12 +550,12 @@ func (b *NetworkManagerBackend) handleDeviceAdded(devicePath dbus.ObjectPath) {
}
hwAddr, _ := w.GetPropertyHwAddress()
b.ethernetDevices[iface] = &ethernetDeviceInfo{
b.setEthernetDeviceInfo(iface, &ethernetDeviceInfo{
device: dev,
wired: w,
name: iface,
hwAddress: hwAddr,
}
})
if b.ethernetDevice == nil {
b.ethernetDevice = dev
@@ -573,12 +573,12 @@ func (b *NetworkManagerBackend) handleDeviceAdded(devicePath dbus.ObjectPath) {
}
hwAddr, _ := w.GetPropertyHwAddress()
b.wifiDevices[iface] = &wifiDeviceInfo{
b.setWifiDeviceInfo(iface, &wifiDeviceInfo{
device: dev,
wireless: w,
name: iface,
hwAddress: hwAddr,
}
})
if b.wifiDevice == nil {
b.wifiDevice = dev
@@ -603,57 +603,49 @@ func (b *NetworkManagerBackend) handleDeviceRemoved(devicePath dbus.ObjectPath)
)
}
for iface, info := range b.ethernetDevices {
if info.device.GetPath() == devicePath {
delete(b.ethernetDevices, iface)
if b.ethernetDevice != nil {
dev := b.ethernetDevice.(gonetworkmanager.Device)
if dev.GetPath() == devicePath {
b.ethernetDevice = nil
for _, remaining := range b.ethernetDevices {
b.ethernetDevice = remaining.device
break
}
if _, remaining, found := b.removeEthernetDeviceByPath(devicePath); found {
if b.ethernetDevice != nil {
dev := b.ethernetDevice.(gonetworkmanager.Device)
if dev.GetPath() == devicePath {
b.ethernetDevice = nil
for _, r := range remaining {
b.ethernetDevice = r.device
break
}
}
b.updateAllEthernetDevices()
b.updateEthernetState()
b.listEthernetConnections()
b.updatePrimaryConnection()
if b.onStateChange != nil {
b.onStateChange()
}
return
}
b.updateAllEthernetDevices()
b.updateEthernetState()
b.listEthernetConnections()
b.updatePrimaryConnection()
if b.onStateChange != nil {
b.onStateChange()
}
return
}
for iface, info := range b.wifiDevices {
if info.device.GetPath() == devicePath {
delete(b.wifiDevices, iface)
if b.wifiDevice != nil {
dev := b.wifiDevice.(gonetworkmanager.Device)
if dev.GetPath() == devicePath {
b.wifiDevice = nil
b.wifiDev = nil
for _, remaining := range b.wifiDevices {
b.wifiDevice = remaining.device
b.wifiDev = remaining.wireless
break
}
if _, remaining, found := b.removeWifiDeviceByPath(devicePath); found {
if b.wifiDevice != nil {
dev := b.wifiDevice.(gonetworkmanager.Device)
if dev.GetPath() == devicePath {
b.wifiDevice = nil
b.wifiDev = nil
for _, r := range remaining {
b.wifiDevice = r.device
b.wifiDev = r.wireless
break
}
}
b.updateAllWiFiDevices()
b.updateWiFiState()
if b.onStateChange != nil {
b.onStateChange()
}
return
}
b.updateAllWiFiDevices()
b.updateWiFiState()
if b.onStateChange != nil {
b.onStateChange()
}
return
}
}
@@ -76,7 +76,7 @@ func (b *NetworkManagerBackend) updateEthernetState() error {
var connectedIP string
var anyConnected bool
for name, info := range b.ethernetDevices {
for name, info := range b.ethernetDevicesSnapshot() {
state, err := info.device.GetPropertyState()
if err != nil {
continue
@@ -973,7 +973,7 @@ func (b *NetworkManagerBackend) SetWiFiAutoconnect(ssid string, autoconnect bool
}
func (b *NetworkManagerBackend) ScanWiFiDevice(device string) error {
devInfo, ok := b.wifiDevices[device]
devInfo, ok := b.wifiDeviceByIface(device)
if !ok {
return fmt.Errorf("WiFi device not found: %s", device)
}
@@ -995,7 +995,7 @@ func (b *NetworkManagerBackend) ScanWiFiDevice(device string) error {
}
func (b *NetworkManagerBackend) DisconnectWiFiDevice(device string) error {
devInfo, ok := b.wifiDevices[device]
devInfo, ok := b.wifiDeviceByIface(device)
if !ok {
return fmt.Errorf("WiFi device not found: %s", device)
}
@@ -1047,7 +1047,7 @@ func (b *NetworkManagerBackend) updateAllWiFiDevices() {
wifiConnected := b.state.WiFiConnected
b.stateMutex.RUnlock()
for name, devInfo := range b.wifiDevices {
for name, devInfo := range b.wifiDevicesSnapshot() {
state, _ := devInfo.device.GetPropertyState()
connected := state == gonetworkmanager.NmDeviceStateActivated
@@ -1211,7 +1211,7 @@ func (b *NetworkManagerBackend) updateAllWiFiDevices() {
func (b *NetworkManagerBackend) getWifiDeviceForConnection(deviceName string) (*wifiDeviceInfo, error) {
if deviceName != "" {
devInfo, ok := b.wifiDevices[deviceName]
devInfo, ok := b.wifiDeviceByIface(deviceName)
if !ok {
return nil, fmt.Errorf("WiFi device not found: %s", deviceName)
}
@@ -1224,7 +1224,7 @@ func (b *NetworkManagerBackend) getWifiDeviceForConnection(deviceName string) (*
dev := b.wifiDevice.(gonetworkmanager.Device)
iface, _ := dev.GetPropertyInterface()
if devInfo, ok := b.wifiDevices[iface]; ok {
if devInfo, ok := b.wifiDeviceByIface(iface); ok {
return devInfo, nil
}
+26 -4
View File
@@ -8,6 +8,7 @@ import (
"net"
"os"
"path/filepath"
"runtime/debug"
"strconv"
"strings"
"sync"
@@ -41,7 +42,7 @@ import (
"github.com/AvengeMedia/DankMaterialShell/core/pkg/syncmap"
)
const APIVersion = 27
const APIVersion = 28
var CLIVersion = "dev"
@@ -398,6 +399,11 @@ func InitializeSysUpdateManager() error {
func handleConnection(conn net.Conn) {
defer conn.Close()
defer func() {
if r := recover(); r != nil {
log.Errorf("handleConnection panic recovered: panic=%v\n%s", r, debug.Stack())
}
}()
caps := getCapabilities()
capsData, _ := json.Marshal(caps)
@@ -415,10 +421,21 @@ func handleConnection(conn net.Conn) {
continue
}
go RouteRequest(conn, req)
go routeRequestRecovered(conn, req)
}
}
// routeRequestRecovered keeps a panicking handler from taking down the whole daemon
func routeRequestRecovered(conn net.Conn, req models.Request) {
defer func() {
if r := recover(); r != nil {
log.Errorf("RouteRequest panic recovered: method=%s panic=%v\n%s", req.Method, r, debug.Stack())
models.RespondError(conn, req.ID, "internal server error")
}
}()
RouteRequest(conn, req)
}
func getCapabilities() Capabilities {
caps := []string{"plugins"}
@@ -581,6 +598,11 @@ func notifyCapabilityChange() {
func handleSubscribe(conn net.Conn, req models.Request) {
clientID := fmt.Sprintf("meta-client-%p", conn)
dbusClient := dbusClientID
if id, ok := models.Get[string](req, "clientId"); ok && id != "" {
dbusClient = id
}
var services []string
if servicesParam, ok := models.Get[[]any](req, "services"); ok {
for _, s := range servicesParam {
@@ -1249,10 +1271,10 @@ func handleSubscribe(conn net.Conn, req models.Request) {
if shouldSubscribe("dbus") && dbusManager != nil {
wg.Add(1)
dbusChan := dbusManager.SubscribeSignals(dbusClientID)
dbusChan := dbusManager.SubscribeSignals(dbusClient)
go func() {
defer wg.Done()
defer dbusManager.UnsubscribeSignals(dbusClientID)
defer dbusManager.UnsubscribeSignals(dbusClient)
for {
select {