Utf8 validation for topics, topic filters, username and client id

awslabs · Aug 23, 2023 · d15eb9e · d15eb9e
1 parent 26eea2f
commit d15eb9e
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 6 deletions.
diff --git a/source/client.c b/source/client.c
@@ -942,18 +942,18 @@ static int s_aws_mqtt_client_connection_311_set_will(
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
+    if (!aws_mqtt_is_valid_topic(topic)) {
+        AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Will topic is invalid", (void *)connection);
+        return aws_raise_error(AWS_ERROR_MQTT_INVALID_TOPIC);
+    }
+
     int result = AWS_OP_ERR;
     AWS_LOGF_TRACE(
         AWS_LS_MQTT_CLIENT,
         "id=%p: Setting last will with topic \"" PRInSTR "\"",
         (void *)connection,
         AWS_BYTE_CURSOR_PRI(*topic));
 
-    if (!aws_mqtt_is_valid_topic(topic)) {
-        AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Will topic is invalid", (void *)connection);
-        return aws_raise_error(AWS_ERROR_MQTT_INVALID_TOPIC);
-    }
-
     struct aws_byte_buf local_topic_buf;
     struct aws_byte_buf local_payload_buf;
     AWS_ZERO_STRUCT(local_topic_buf);
@@ -1007,6 +1007,11 @@ static int s_aws_mqtt_client_connection_311_set_login(
         return aws_raise_error(AWS_ERROR_INVALID_STATE);
     }
 
+    if (username != NULL && aws_mqtt_validate_utf8_text(*username) == AWS_OP_ERR) {
+        AWS_LOGF_DEBUG(AWS_LS_MQTT_CLIENT, "id=%p: Invalid utf8 or forbidden codepoints in username", (void *)connection);
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
     int result = AWS_OP_ERR;
     AWS_LOGF_TRACE(AWS_LS_MQTT_CLIENT, "id=%p: Setting username and password", (void *)connection);
 
@@ -1428,6 +1433,15 @@ static int s_aws_mqtt_client_connection_311_connect(
 
     struct aws_mqtt_client_connection_311_impl *connection = impl;
 
+    if (connection_options == NULL) {
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    if (aws_mqtt_validate_utf8_text(connection_options->client_id) == AWS_OP_ERR) {
+        AWS_LOGF_DEBUG(AWS_LS_MQTT_CLIENT, "id=%p: Invalid utf8 or forbidden codepoints in client id", (void *)connection);
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
     /* TODO: Do we need to support resuming the connection if user connect to the same connection & endpoint and the
      * clean_session is false?
      * If not, the broker will resume the connection in this case, and we pretend we are making a new connection, which

diff --git a/source/mqtt.c b/source/mqtt.c
@@ -23,6 +23,10 @@ static bool s_is_valid_topic(const struct aws_byte_cursor *topic, bool is_filter
         return false;
     }
 
+    if (aws_mqtt_validate_utf8_text(*topic) == AWS_OP_ERR) {
+        return false;
+    }
+
     /* [MQTT-4.7.3-2] Check for the null character */
     if (memchr(topic->ptr, 0, topic->len)) {
         return false;

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -90,6 +90,9 @@ add_test_case(mqtt_connection_reconnection_backoff_reset_after_disconnection)
 add_test_case(mqtt_validation_failure_publish_qos)
 add_test_case(mqtt_validation_failure_subscribe_empty)
 add_test_case(mqtt_validation_failure_unsubscribe_null)
+add_test_case(mqtt_validation_failure_connect_invalid_client_id_utf8)
+add_test_case(mqtt_validation_failure_invalid_will_topic_utf8)
+add_test_case(mqtt_validation_failure_invalid_username_utf8)
 
 # Operation statistics tests
 add_test_case(mqtt_operation_statistics_simple_publish)

diff --git a/tests/shared_utils.c → tests/shared_utils_tests.c b/tests/shared_utils.c → tests/shared_utils_tests.c
diff --git a/tests/v3/connection_state_test.c b/tests/v3/connection_state_test.c
@@ -3868,4 +3868,73 @@ AWS_TEST_CASE_FIXTURE(
     s_setup_mqtt_server_fn,
     s_test_mqtt_validation_failure_unsubscribe_null_fn,
     s_clean_up_mqtt_server_fn,
-    &test_data)
+    &test_data)
+
+static struct aws_byte_cursor s_bad_client_id_utf8 = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("\x41\xED\xA0\x80\x41");
+static struct aws_byte_cursor s_bad_username_utf8 = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("\x41\x00\x41");
+static struct aws_byte_cursor s_bad_will_topic_utf8 = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("\x41\xED\xBF\xBF");
+
+static int s_test_mqtt_validation_failure_connect_invalid_client_id_utf8_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    struct mqtt_connection_state_test *state_test_data = ctx;
+
+    struct aws_mqtt_connection_options connection_options = {
+        .user_data = state_test_data,
+        .clean_session = false,
+        .client_id = s_bad_client_id_utf8,
+        .host_name = aws_byte_cursor_from_c_str(state_test_data->endpoint.address),
+        .socket_options = &state_test_data->socket_options,
+        .on_connection_complete = s_on_connection_complete_fn,
+    };
+
+    ASSERT_FAILS(aws_mqtt_client_connection_connect(state_test_data->mqtt_connection, &connection_options));
+    int error_code = aws_last_error();
+    ASSERT_INT_EQUALS(AWS_ERROR_INVALID_ARGUMENT, error_code);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE_FIXTURE(
+    mqtt_validation_failure_connect_invalid_client_id_utf8,
+    s_setup_mqtt_server_fn,
+    s_test_mqtt_validation_failure_connect_invalid_client_id_utf8_fn,
+    s_clean_up_mqtt_server_fn,
+    &test_data)
+
+static int s_test_mqtt_validation_failure_invalid_will_topic_utf8_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    struct mqtt_connection_state_test *state_test_data = ctx;
+
+    struct aws_byte_cursor will_topic_cursor = s_bad_will_topic_utf8;
+    ASSERT_FAILS(aws_mqtt_client_connection_set_will(state_test_data->mqtt_connection, &will_topic_cursor, AWS_MQTT_QOS_AT_MOST_ONCE, false, &will_topic_cursor));
+    int error_code = aws_last_error();
+    ASSERT_INT_EQUALS(AWS_ERROR_MQTT_INVALID_TOPIC, error_code);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE_FIXTURE(
+    mqtt_validation_failure_invalid_will_topic_utf8,
+    s_setup_mqtt_server_fn,
+    s_test_mqtt_validation_failure_invalid_will_topic_utf8_fn,
+    s_clean_up_mqtt_server_fn,
+    &test_data)
+
+static int s_test_mqtt_validation_failure_invalid_username_utf8_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    struct mqtt_connection_state_test *state_test_data = ctx;
+
+    struct aws_byte_cursor login_cursor = s_bad_username_utf8;
+    ASSERT_FAILS(aws_mqtt_client_connection_set_login(state_test_data->mqtt_connection, &login_cursor, NULL));
+    int error_code = aws_last_error();
+    ASSERT_INT_EQUALS(AWS_ERROR_INVALID_ARGUMENT, error_code);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE_FIXTURE(
+    mqtt_validation_failure_invalid_username_utf8,
+    s_setup_mqtt_server_fn,
+    s_test_mqtt_validation_failure_invalid_username_utf8_fn,
+    s_clean_up_mqtt_server_fn,
+    &test_data)
diff --git a/tests/v3/topic_tree_test.c b/tests/v3/topic_tree_test.c
@@ -334,6 +334,8 @@ static int s_mqtt_topic_validation_fn(struct aws_allocator *allocator, void *ctx
     ASSERT_TOPIC_VALIDITY(FALSE, "sport/+/player1");
     ASSERT_TOPIC_VALIDITY(FALSE, "sport+");
 
+    ASSERT_TOPIC_VALIDITY(FALSE, "\x41/\xED\xBF\xBF/\x41");
+
     return AWS_OP_SUCCESS;
 }
 
@@ -362,5 +364,7 @@ static int s_mqtt_topic_filter_validation_fn(struct aws_allocator *allocator, vo
     ASSERT_TOPIC_FILTER_VALIDITY(TRUE, "sport/+/player1");
     ASSERT_TOPIC_FILTER_VALIDITY(FALSE, "sport+");
 
+    ASSERT_TOPIC_FILTER_VALIDITY(FALSE, "\x41/\xED\xA0\x80/\x41");
+
     return AWS_OP_SUCCESS;
 }