Merge branch 'coredevices:main' into copilot/add-quick-launch-shortcut-shutdown

Author: s00se

docs/development/qemu.md

@@ -29,10 +29,10 @@ You can launch QEMU with the built image using:
 ./waf qemu
 ```
 
-The flash image is built automatically the first time. To force rebuilding it, pass `--new-flash-image`:
+The flash image is rebuilt by default on every launch. To keep the existing flash image (e.g. to preserve stored apps), pass `--keep-flash-image`:
 
 ```shell
-./waf qemu --new-flash-image
+./waf qemu --keep-flash-image
 ```
 
 ## Console

include/pbl/services/event_service.h

@@ -29,3 +29,9 @@ void event_service_clear_process_subscriptions(PebbleTask task);
 void* event_service_claim_buffer(PebbleEvent *e);
 //! This function expects the pointer returned by event_service_claim_buffer
 void event_service_free_claimed_buffer(void *ref);
+
+//! Returns true if `buf` is a kernel-allocated buffer currently tracked by the
+//! event service (i.e. one that was attached to an in-flight PebbleEvent).
+//! Use this from syscalls that dereference an event's embedded data pointer to
+//! make sure the caller didn't fabricate it.
+bool event_service_is_known_buffer(const void *buf);

src/fw/applib/bluetooth/ble_client.c

@@ -6,6 +6,7 @@
 #include "ble_app_support.h"
 
 #include "applib/applib_malloc.auto.h"
+#include "pbl/services/event_service.h"
 #include "process_state/app_state/app_state.h"
 #include "syscall/syscall.h"
 #include "syscall/syscall_internal.h"
@@ -44,8 +45,18 @@ static void prv_handle_services_added(
 DEFINE_SYSCALL(void, sys_get_service_discovery_info, const PebbleBLEGATTClientServiceEvent *e,
                PebbleBLEGATTClientServiceEventInfo *info) {
   if (PRIVILEGE_WAS_ELEVATED) {
+    syscall_assert_userspace_buffer(e, sizeof(*e));
     // Note: if we start storing services, we will need to update the size
     syscall_assert_userspace_buffer(info, sizeof(*info));
+    // `e->info` is a kernel pointer the event service attached to the event
+    // when it was posted; the app can rewrite it in its local copy before
+    // calling us. Confirm it still references a kernel-tracked event buffer
+    // — without that, an app could aim it at any kernel address and read
+    // arbitrary kernel bytes back via *info.
+    if (!event_service_is_known_buffer(e->info)) {
+      PBL_LOG_ERR("Rejecting service event with unknown info pointer %p", e->info);
+      syscall_failed();
+    }
   }
 
   *info = (PebbleBLEGATTClientServiceEventInfo) {

src/fw/applib/pbl_std/pbl_std.c

@@ -4,11 +4,13 @@
 #include "applib/pbl_std/pbl_std.h"
 #include "applib/app_logging.h"
 #include "applib/applib_malloc.auto.h"
+#include "kernel/memory_layout.h"
 #include "util/time/time.h"
 #include "process_state/app_state/app_state.h"
 #include "process_state/worker_state/worker_state.h"
 #include "syscall/syscall.h"
 #include "syscall/syscall_internal.h"
+#include "system/logging.h"
 
 // Time
 time_t pbl_override_time(time_t *tloc) {
@@ -203,12 +205,24 @@ size_t pbl_strftime(char* s, size_t maxsize, const char* format, const struct tm
   return sys_strftime(s, maxsize, format, tim_p, locale);
 }
 
+// Cap on how far we'll scan for the format-string terminator before giving up
+// — well beyond any reasonable strftime format.
+#define SYS_STRFTIME_FORMAT_MAX 256
+
 DEFINE_SYSCALL(size_t, sys_strftime, char* s, size_t maxsize, const char* format,
                                   const struct tm* tim_p, char *locale) {
 
   if (PRIVILEGE_WAS_ELEVATED) {
     syscall_assert_userspace_buffer(s, maxsize);
-    syscall_assert_userspace_buffer(format, strlen(format));
+    // Verify `format` is a null-terminated string entirely within the caller's
+    // app region BEFORE calling strlen() on it; otherwise an app could hand us
+    // a kernel pointer and have strlen() dereference whatever it pleases
+    // (faulting on unmapped memory, or running away on missing terminators).
+    if (!memory_layout_is_cstring_in_region(memory_layout_get_app_region(), format,
+                                            SYS_STRFTIME_FORMAT_MAX)) {
+      PBL_LOG_ERR("strftime format %p not in app region", format);
+      syscall_failed();
+    }
     syscall_assert_userspace_buffer(tim_p, sizeof(struct tm));
   }
 

src/fw/applib/voice/voice_window.c

@@ -42,6 +42,7 @@
 #include "process_management/app_manager.h"
 #include "resource/resource_ids.auto.h"
 #include "pbl/services/comm_session/session.h"
+#include "pbl/services/event_service.h"
 #include "pbl/services/voice/voice.h"
 #include "syscall/syscall.h"
 #include "system/logging.h"
@@ -1432,6 +1433,15 @@ DEFINE_SYSCALL(char *, sys_voice_get_transcription_from_event, PebbleVoiceServic
       syscall_assert_userspace_buffer(buffer, buffer_size);
     }
     syscall_assert_userspace_buffer(sentence_len, sizeof(*sentence_len));
+    // `e->data` is a kernel pointer the event service handed to the app along
+    // with the event. The app can rewrite the field in its copy before calling
+    // us, so confirm it still references a buffer the kernel actually allocated
+    // — otherwise an app could aim it at kernel memory and turn the strlen/memcpy
+    // below into an arbitrary kernel read primitive.
+    if (!event_service_is_known_buffer(e->data)) {
+      PBL_LOG_ERR("Rejecting voice event with unknown data pointer %p", e->data);
+      syscall_failed();
+    }
   }
 
   size_t len;

src/fw/services/accel_manager/service.c

@@ -462,6 +462,20 @@ DEFINE_SYSCALL(AccelManagerState*, sys_accel_manager_data_subscribe,
                PebbleTask handler_task) {
   AccelManagerState *state;
 
+  // `handler_task` decides where prv_call_data_callback() dispatches the
+  // user-supplied data_cb. For KernelMain/KernelBackground/NewTimers values
+  // the callback ends up invoked directly in kernel mode (either via the
+  // kernel main event loop's PEBBLE_CALLBACK_EVENT handler, system_task_add_callback,
+  // or new_timer_add_work_callback) — handing an unprivileged app arbitrary
+  // kernel-mode code execution. Force unprivileged callers onto their own
+  // task so the dispatch lands in their app/worker event loop instead.
+  if (PRIVILEGE_WAS_ELEVATED) {
+    handler_task = pebble_task_get_current();
+    if (handler_task != PebbleTask_App && handler_task != PebbleTask_Worker) {
+      syscall_failed();
+    }
+  }
+
   mutex_lock_recursive(s_accel_manager_mutex);
   {
     state = kernel_malloc_check(sizeof(AccelManagerState));

src/fw/services/activity/activity.c

@@ -1102,12 +1102,27 @@ bool activity_get_sessions(uint32_t *session_entries, ActivitySession *sessions)
 DEFINE_SYSCALL(bool, sys_activity_get_sessions, uint32_t *session_entries,
                ActivitySession *sessions) {
   if (PRIVILEGE_WAS_ELEVATED) {
-    if (session_entries) {
-      syscall_assert_userspace_buffer(session_entries, sizeof(*session_entries));
+    if (!session_entries) {
+      // The implementation dereferences session_entries unconditionally.
+      return false;
+    }
+    syscall_assert_userspace_buffer(session_entries, sizeof(*session_entries));
+    // Same pattern as sys_activity_get_minute_history: snapshot *session_entries
+    // so the inner activity_get_sessions can't race us into a larger memcpy than
+    // we validated, and reject sizes whose `requested * sizeof(*sessions)` would
+    // wrap and let a much-smaller validated buffer gate a much-larger write.
+    const uint32_t requested = *session_entries;
+    if (requested > SIZE_MAX / sizeof(*sessions)) {
+      PBL_LOG_ERR("session_entries=%" PRIu32 " would overflow size", requested);
+      syscall_failed();
     }
     if (sessions) {
-      syscall_assert_userspace_buffer(sessions, sizeof(*sessions) * (*session_entries));
+      syscall_assert_userspace_buffer(sessions, requested * sizeof(*sessions));
     }
+    uint32_t entries_inout = requested;
+    bool result = activity_get_sessions(&entries_inout, sessions);
+    *session_entries = entries_inout;
+    return result;
   }
 
   return activity_get_sessions(session_entries, sessions);
@@ -1176,7 +1191,24 @@ DEFINE_SYSCALL(bool, sys_activity_get_minute_history, HealthMinuteData *minute_d
   if (PRIVILEGE_WAS_ELEVATED) {
     syscall_assert_userspace_buffer(utc_start, sizeof(*utc_start));
     syscall_assert_userspace_buffer(num_records, sizeof(*num_records));
-    syscall_assert_userspace_buffer(minute_data, *num_records * sizeof(HealthMinuteData));
+    // Snapshot the requested count to a kernel-stack local so that:
+    //  - the size computation below can't be tricked by an app racing the
+    //    KernelBG callback to swap *num_records after we validated it (TOCTOU);
+    //  - we can bound the multiplication against SIZE_MAX before performing it
+    //    (otherwise `*num_records * sizeof(HealthMinuteData)` wraps in uint32_t
+    //    and a tiny validated size would gate a much larger kernel write).
+    const uint32_t requested = *num_records;
+    if (requested > SIZE_MAX / sizeof(HealthMinuteData)) {
+      PBL_LOG_ERR("num_records=%" PRIu32 " would overflow size", requested);
+      syscall_failed();
+    }
+    syscall_assert_userspace_buffer(minute_data, requested * sizeof(HealthMinuteData));
+    uint32_t records_inout = requested;
+    time_t start_inout = *utc_start;
+    bool result = activity_get_minute_history(minute_data, &records_inout, &start_inout);
+    *num_records = records_inout;
+    *utc_start = start_inout;
+    return result;
   }
 
   return activity_get_minute_history(minute_data, num_records, utc_start);

src/fw/services/activity/activity_metrics.c

@@ -784,6 +784,17 @@ bool activity_get_metric(ActivityMetric metric, uint32_t history_len, int32_t *h
 DEFINE_SYSCALL(bool, sys_activity_get_metric, ActivityMetric metric,
                uint32_t history_len, int32_t *history) {
   if (PRIVILEGE_WAS_ELEVATED) {
+    // activity_get_metric() unconditionally writes history_len int32_t entries
+    // to `history` before later clamping to ACTIVITY_HISTORY_DAYS. A huge
+    // history_len makes `history_len * sizeof(*history)` wrap to a tiny size
+    // that passes validation while the kernel keeps writing -1 past the end
+    // (effectively an arbitrary-address kernel write).
+    //
+    // We never read more than ACTIVITY_HISTORY_DAYS entries anyway, so clamp
+    // the count here and validate against the clamped value.
+    if (history_len > ACTIVITY_HISTORY_DAYS) {
+      history_len = ACTIVITY_HISTORY_DAYS;
+    }
     if (history) {
       syscall_assert_userspace_buffer(history, history_len * sizeof(*history));
     }

src/fw/services/analytics/analytics.c

@@ -107,22 +107,42 @@ void pbl_analytics_init(void) {
                   TIMER_START_FLAG_REPEATING);
 }
 
+// Apps pass `key` straight through to backends that use it as an unchecked
+// index into fixed-size kernel tables (`s_key_to_integer[]`, `s_pbl_to_memfault[]`,
+// `s_string_ptrs[]`, `s_string_lens[]`, ...). An out-of-range key reads
+// arbitrary kernel bytes; in the set_string path those bytes become a kernel
+// pointer + length that strncpy() then writes the (validated) app string into,
+// turning analytics into an arbitrary kernel write primitive. Bound the key
+// here once and let the backends keep their direct indexing.
+static bool prv_analytics_key_in_range(enum pbl_analytics_key key) {
+  return ((unsigned)key < PBL_ANALYTICS_KEY_COUNT);
+}
+
 DEFINE_SYSCALL(void, sys_pbl_analytics_set_signed, enum pbl_analytics_key key,
                int32_t signed_value) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   for (size_t i = 0U; i < ARRAY_LENGTH(s_backend_ops); i++) {
     s_backend_ops[i]->set_signed(key, signed_value);
   }
 }
 
 DEFINE_SYSCALL(void, sys_pbl_analytics_set_unsigned, enum pbl_analytics_key key,
                uint32_t unsigned_value) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   for (size_t i = 0U; i < ARRAY_LENGTH(s_backend_ops); i++) {
     s_backend_ops[i]->set_unsigned(key, unsigned_value);
   }
 }
 
 DEFINE_SYSCALL(void, sys_pbl_analytics_set_string, enum pbl_analytics_key key,
                const char *value) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   if (PRIVILEGE_WAS_ELEVATED) {
     if (!memory_layout_is_cstring_in_region(
           memory_layout_get_app_region(), value, ANALYTICS_STRING_MAX_LEN)) {
@@ -136,18 +156,27 @@ DEFINE_SYSCALL(void, sys_pbl_analytics_set_string, enum pbl_analytics_key key,
 }
 
 DEFINE_SYSCALL(void, sys_pbl_analytics_timer_start, enum pbl_analytics_key key) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   for (size_t i = 0U; i < ARRAY_LENGTH(s_backend_ops); i++) {
     s_backend_ops[i]->timer_start(key);
   }
 }
 
 DEFINE_SYSCALL(void, sys_pbl_analytics_timer_stop, enum pbl_analytics_key key) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   for (size_t i = 0U; i < ARRAY_LENGTH(s_backend_ops); i++) {
     s_backend_ops[i]->timer_stop(key);
   }
 }
 
 DEFINE_SYSCALL(void, sys_pbl_analytics_add, enum pbl_analytics_key key, int32_t amount) {
+  if (!prv_analytics_key_in_range(key)) {
+    return;
+  }
   for (size_t i = 0U; i < ARRAY_LENGTH(s_backend_ops); i++) {
     s_backend_ops[i]->add(key, amount);
   }

src/fw/services/data_logging/dls_syscalls.c

@@ -23,12 +23,9 @@ DEFINE_SYSCALL(DataLoggingSessionRef, sys_data_logging_create, uint32_t tag,
 }
 
 DEFINE_SYSCALL(void, sys_data_logging_finish, DataLoggingSessionRef session_ref) {
-  // TODO: It would be nice to verify the session itself, because they could be
-  // passing us any memory address (not necesarilly a valid DataLoggingSession).
-  // An evil developer could potentially use this to confuse the data_logging
-  // logic, and do evil things with kernel rights. However, it's pretty unlikely
-  // (especially since our executable code lives in microflash, and hence can't
-  // just be overwritten by a buffer overrun), so it's probably fine.
+  // dls_is_session_valid() (below) walks the kernel-owned s_logging_sessions
+  // list to confirm the session pointer is one we handed out, so a forged
+  // session_ref simply returns invalid and never reaches dls_finish().
   DataLoggingSession* session = (DataLoggingSession*)session_ref;
 
   if (!dls_is_session_valid(session)) {