mqtt: de-permission publish callback capabilities.

The `topic` and `payload` capabilities of the publish callback are only valid within the context of the callback. They should thus passed as a read-only, non-capturable capabilities. Currently we pass them as capturable and writable capabilities, which may allow API users to compromise the MQTT compartment. This addresses issue #43. Signed-off-by: Hugo Lefeuvre <[email protected]>
CHERIoT-Platform · Oct 22, 2024 · d216e33 · d216e33
1 parent a9a540b
commit d216e33
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 3 deletions.
diff --git a/include/mqtt.h b/include/mqtt.h
@@ -11,7 +11,9 @@
  * will be called on all PUBLISH notifications from the broker.
  *
  * `topicName` and `payload` (and their respective size arguments) indicate the
- * topic of the PUBLISH, and the corresponding payload.
+ * topic of the PUBLISH, and the corresponding payload. Both are only valid
+ * within the context of the callback and thus passed as a read-only,
+ * non-capturable capabilities.
  */
 typedef void __cheri_callback (*MQTTPublishCallback)(const char *topicName,
                                                      size_t topicNameLength,

diff --git a/lib/mqtt/mqtt.cc b/lib/mqtt/mqtt.cc
@@ -468,9 +468,17 @@ namespace
 			              "The packet is of type PUBLISH, but topic or payload "
 			              "are not set.");
 
-			publishCallback(publishInfo->pTopicName,
+			// The payload and topic are only valid within the
+			// context of the callback: make them read-only and
+			// non-capturable.
+			Capability topic{publishInfo->pTopicName};
+			Capability payload{publishInfo->pPayload};
+			topic.permissions() &= CHERI::Permission::Load;
+			payload.permissions() &= CHERI::Permission::Load;
+
+			publishCallback(topic,
 			                publishInfo->topicNameLength,
-			                publishInfo->pPayload,
+			                payload,
 			                publishInfo->payloadLength);
 		}
 		else if (ackCallback)