From f9e0fa246a1e3b79611b229fcdffc28559c587e8 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Thu, 1 Nov 2018 18:53:06 +0000 Subject: [PATCH] Validate properties coming into client library. --- client/client_props.c | 2 +- client/client_shared.c | 36 ++++++++++++ lib/actions.c | 24 ++++++++ lib/connect.c | 13 +++++ lib/linker.version | 3 +- lib/mosquitto.c | 2 + lib/mosquitto.h | 39 ++++++++++++- lib/options.c | 9 +++ lib/property_mosq.c | 123 ++++++++++++++++++++--------------------- lib/will_mosq.c | 2 +- 10 files changed, 184 insertions(+), 69 deletions(-) diff --git a/client/client_props.c b/client/client_props.c index 1e641514..71dcc2d4 100644 --- a/client/client_props.c +++ b/client/client_props.c @@ -85,7 +85,7 @@ int cfg_parse_property(struct mosq_config *cfg, int argc, char *argv[], int *idx return MOSQ_ERR_INVAL; } - if(mosquitto_property_command_check(cmd, identifier)){ + if(mosquitto_property_check_command(cmd, identifier)){ fprintf(stderr, "Error: %s property not allow for %s in --property argument.\n\n", propname, cmdname); return MOSQ_ERR_INVAL; } diff --git a/client/client_shared.c b/client/client_shared.c index 41fefa90..fb1ba310 100644 --- a/client/client_shared.c +++ b/client/client_shared.c @@ -32,6 +32,7 @@ Contributors: #endif #include +#include #include "client_shared.h" static int mosquitto__parse_socks_url(struct mosq_config *cfg, char *url); @@ -184,6 +185,7 @@ void client_config_cleanup(struct mosq_config *cfg) mosquitto_property_free_all(&cfg->subscribe_props); mosquitto_property_free_all(&cfg->unsubscribe_props); mosquitto_property_free_all(&cfg->disconnect_props); + mosquitto_property_free_all(&cfg->will_props); } int client_config_load(struct mosq_config *cfg, int pub_or_sub, int argc, char *argv[]) @@ -344,6 +346,38 @@ int client_config_load(struct mosq_config *cfg, int pub_or_sub, int argc, char * return 1; } } + + rc = mosquitto_property_check_all(CMD_CONNECT, cfg->connect_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in CONNECT properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + rc = mosquitto_property_check_all(CMD_PUBLISH, cfg->publish_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in PUBLISH properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + rc = mosquitto_property_check_all(CMD_SUBSCRIBE, cfg->subscribe_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in SUBSCRIBE properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + rc = mosquitto_property_check_all(CMD_UNSUBSCRIBE, cfg->unsubscribe_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in UNSUBSCRIBE properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + rc = mosquitto_property_check_all(CMD_DISCONNECT, cfg->disconnect_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in DISCONNECT properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + rc = mosquitto_property_check_all(CMD_WILL, cfg->will_props); + if(rc){ + if(!cfg->quiet) fprintf(stderr, "Error in Will properties: %s\n", mosquitto_strerror(rc)); + return 1; + } + return MOSQ_ERR_SUCCESS; } @@ -918,6 +952,8 @@ int client_opts_set(struct mosquitto *mosq, struct mosq_config *cfg) mosquitto_lib_cleanup(); return 1; } + cfg->will_props = NULL; + if(cfg->username && mosquitto_username_pw_set(mosq, cfg->username, cfg->password)){ if(!cfg->quiet) fprintf(stderr, "Error: Problem setting username and password.\n"); mosquitto_lib_cleanup(); diff --git a/lib/actions.c b/lib/actions.c index 35826971..e0ccf0f3 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -40,6 +40,7 @@ int mosquitto_publish_with_properties(struct mosquitto *mosq, int *mid, const ch int queue_status; const mosquitto_property *p; bool have_topic_alias; + int rc; if(!mosq || qos<0 || qos>2) return MOSQ_ERR_INVAL; if(!topic || STREMPTY(topic)){ @@ -68,6 +69,10 @@ int mosquitto_publish_with_properties(struct mosquitto *mosq, int *mid, const ch return MOSQ_ERR_INVAL; } } + if(properties){ + rc = mosquitto_property_check_all(CMD_PUBLISH, properties); + if(rc) return rc; + } local_mid = mosquitto__mid_generate(mosq); if(mid){ @@ -132,12 +137,19 @@ int mosquitto_subscribe(struct mosquitto *mosq, int *mid, const char *sub, int q int mosquitto_subscribe_with_properties(struct mosquitto *mosq, int *mid, const char *sub, int qos, const mosquitto_property *properties) { + int rc; + if(!mosq) return MOSQ_ERR_INVAL; if(mosq->sock == INVALID_SOCKET) return MOSQ_ERR_NO_CONN; if(mosquitto_sub_topic_check(sub)) return MOSQ_ERR_INVAL; if(mosquitto_validate_utf8(sub, strlen(sub))) return MOSQ_ERR_MALFORMED_UTF8; + if(properties){ + rc = mosquitto_property_check_all(CMD_SUBSCRIBE, properties); + if(rc) return rc; + } + return send__subscribe(mosq, mid, 1, (char *const *const)&sub, qos, properties); } @@ -145,11 +157,17 @@ int mosquitto_subscribe_with_properties(struct mosquitto *mosq, int *mid, const int mosquitto_subscribe_multiple(struct mosquitto *mosq, int *mid, int sub_count, char *const *const sub, int qos, const mosquitto_property *properties) { int i; + int rc; if(!mosq || !sub_count || !sub) return MOSQ_ERR_INVAL; if(qos < 0 || qos > 2) return MOSQ_ERR_INVAL; if(mosq->sock == INVALID_SOCKET) return MOSQ_ERR_NO_CONN; + if(mosq->protocol == mosq_p_mqtt5 && properties){ + rc = mosquitto_property_check_all(CMD_SUBSCRIBE, properties); + if(rc) return rc; + } + for(i=0; isock == INVALID_SOCKET) return MOSQ_ERR_NO_CONN; if(mosquitto_sub_topic_check(sub)) return MOSQ_ERR_INVAL; if(mosquitto_validate_utf8(sub, strlen(sub))) return MOSQ_ERR_MALFORMED_UTF8; + if(properties){ + rc = mosquitto_property_check_all(CMD_PUBLISH, properties); + if(rc) return rc; + } return send__unsubscribe(mosq, mid, sub, properties); } diff --git a/lib/connect.c b/lib/connect.c index 88eecca3..100d7e9f 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -22,6 +22,7 @@ Contributors: #include "messages_mosq.h" #include "memory_mosq.h" #include "packet_mosq.h" +#include "mqtt_protocol.h" #include "net_mosq.h" #include "send_mosq.h" #include "socks_mosq.h" @@ -80,6 +81,12 @@ int mosquitto_connect_bind(struct mosquitto *mosq, const char *host, int port, i int mosquitto_connect_bind_with_properties(struct mosquitto *mosq, const char *host, int port, int keepalive, const char *bind_address, const mosquitto_property *properties) { int rc; + + if(properties){ + rc = mosquitto_property_check_all(CMD_CONNECT, properties); + if(rc) return rc; + } + rc = mosquitto__connect_init(mosq, host, port, keepalive, bind_address); if(rc) return rc; @@ -210,8 +217,14 @@ int mosquitto_disconnect(struct mosquitto *mosq) int mosquitto_disconnect_with_properties(struct mosquitto *mosq, const mosquitto_property *properties) { + int rc; if(!mosq) return MOSQ_ERR_INVAL; + if(properties){ + rc = mosquitto_property_check_all(CMD_DISCONNECT, properties); + if(rc) return rc; + } + pthread_mutex_lock(&mosq->state_mutex); mosq->state = mosq_cs_disconnecting; pthread_mutex_unlock(&mosq->state_mutex); diff --git a/lib/linker.version b/lib/linker.version index 499aa303..ccebe673 100644 --- a/lib/linker.version +++ b/lib/linker.version @@ -104,7 +104,8 @@ MOSQ_1.6 { mosquitto_property_add_string_pair; mosquitto_property_add_varint; mosquitto_property_free_all; - mosquitto_property_command_check; + mosquitto_property_check_command; + mosquitto_property_check_all; mosquitto_publish_with_properties; mosquitto_string_to_command; mosquitto_string_to_property_info; diff --git a/lib/mosquitto.c b/lib/mosquitto.c index ebecbdc9..be449fbe 100644 --- a/lib/mosquitto.c +++ b/lib/mosquitto.c @@ -373,6 +373,8 @@ const char *mosquitto_strerror(int mosq_errno) return "Proxy error."; case MOSQ_ERR_MALFORMED_UTF8: return "Malformed UTF-8"; + case MOSQ_ERR_DUPLICATE_PROPERTY: + return "Duplicate property in property list"; default: return "Unknown error."; } diff --git a/lib/mosquitto.h b/lib/mosquitto.h index b5eba8af..9dccb3df 100644 --- a/lib/mosquitto.h +++ b/lib/mosquitto.h @@ -89,6 +89,7 @@ enum mosq_err_t { MOSQ_ERR_KEEPALIVE = 19, MOSQ_ERR_LOOKUP = 20, MOSQ_ERR_MALFORMED_PACKET = 19, + MOSQ_ERR_DUPLICATE_PROPERTY = 20, }; /* Error values */ @@ -347,6 +348,7 @@ libmosq_EXPORT int mosquitto_will_set(struct mosquitto *mosq, const char *topic, * MOSQ_ERR_NOT_SUPPORTED - if properties is not NULL and the client is not * using MQTT v5 * MOSQ_ERR_PROTOCOL - if a property is invalid for use with wills. + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. */ libmosq_EXPORT int mosquitto_will_set_with_properties(struct mosquitto *mosq, const char *topic, int payloadlen, const void *payload, int qos, bool retain, mosquitto_property *properties); @@ -483,6 +485,8 @@ libmosq_EXPORT int mosquitto_connect_bind(struct mosquitto *mosq, const char *ho * contains the error code, even on Windows. * Use strerror_r() where available or FormatMessage() on * Windows. + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid for use with CONNECT. * * See Also: * , , @@ -687,6 +691,8 @@ libmosq_EXPORT int mosquitto_disconnect(struct mosquitto *mosq); * MOSQ_ERR_SUCCESS - on success. * MOSQ_ERR_INVAL - if the input parameters were invalid. * MOSQ_ERR_NO_CONN - if the client isn't connected to a broker. + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid for use with DISCONNECT. */ libmosq_EXPORT int mosquitto_disconnect_with_properties(struct mosquitto *mosq, const mosquitto_property *properties); @@ -774,6 +780,8 @@ libmosq_EXPORT int mosquitto_publish(struct mosquitto *mosq, int *mid, const cha * broker. * MOSQ_ERR_PAYLOAD_SIZE - if payloadlen is too large. * MOSQ_ERR_MALFORMED_UTF8 - if the topic is not valid UTF-8 + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid for use with PUBLISH. */ libmosq_EXPORT int mosquitto_publish_with_properties( struct mosquitto *mosq, @@ -837,6 +845,8 @@ libmosq_EXPORT int mosquitto_subscribe(struct mosquitto *mosq, int *mid, const c * MOSQ_ERR_NOMEM - if an out of memory condition occurred. * MOSQ_ERR_NO_CONN - if the client isn't connected to a broker. * MOSQ_ERR_MALFORMED_UTF8 - if the topic is not valid UTF-8 + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid for use with SUBSCRIBE. */ libmosq_EXPORT int mosquitto_subscribe_with_properties(struct mosquitto *mosq, int *mid, const char *sub, int qos, const mosquitto_property *properties); @@ -913,6 +923,8 @@ libmosq_EXPORT int mosquitto_unsubscribe(struct mosquitto *mosq, int *mid, const * MOSQ_ERR_NOMEM - if an out of memory condition occurred. * MOSQ_ERR_NO_CONN - if the client isn't connected to a broker. * MOSQ_ERR_MALFORMED_UTF8 - if the topic is not valid UTF-8 + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid for use with UNSUBSCRIBE. */ libmosq_EXPORT int mosquitto_unsubscribe_with_properties(struct mosquitto *mosq, int *mid, const char *sub, const mosquitto_property *properties); @@ -2285,7 +2297,7 @@ libmosq_EXPORT int mosquitto_property_add_string_pair(mosquitto_property **propl libmosq_EXPORT void mosquitto_property_free_all(mosquitto_property **properties); /* - * Function: mosquitto_property_command_check + * Function: mosquitto_property_check_command * * Check whether a property identifier is valid for the given command. * @@ -2297,7 +2309,30 @@ libmosq_EXPORT void mosquitto_property_free_all(mosquitto_property **properties) * MOSQ_ERR_SUCCESS - if the identifier is valid for command * MOSQ_ERR_PROTOCOL - if the identifier is not valid for use with command. */ -libmosq_EXPORT int mosquitto_property_command_check(int command, int identifier); +libmosq_EXPORT int mosquitto_property_check_command(int command, int identifier); + + +/* + * Function: mosquitto_property_check_all + * + * Check whether a list of properties are valid for a particular command, + * whether there are duplicates, and whether the values are valid where + * possible. + * + * Note that this function is used internally in the library whenever + * properties are passed to it, so in basic use this is not needed, but should + * be helpful to check property lists *before* the point of using them. + * + * Parameters: + * command - MQTT command (e.g. CMD_CONNECT) + * properties - list of MQTT properties to check. + * + * Returns: + * MOSQ_ERR_SUCCESS - if all properties are valid + * MOSQ_ERR_DUPLICATE_PROPERTY - if a property is duplicated where it is forbidden. + * MOSQ_ERR_PROTOCOL - if any property is invalid + */ +libmosq_EXPORT int mosquitto_property_check_all(int command, const mosquitto_property *properties); /* Function: mosquitto_string_to_property_info * diff --git a/lib/options.c b/lib/options.c index 12a19d8b..4a66f309 100644 --- a/lib/options.c +++ b/lib/options.c @@ -25,6 +25,7 @@ Contributors: #include "mosquitto.h" #include "mosquitto_internal.h" #include "memory_mosq.h" +#include "mqtt_protocol.h" #include "util_mosq.h" #include "will_mosq.h" @@ -37,7 +38,15 @@ int mosquitto_will_set(struct mosquitto *mosq, const char *topic, int payloadlen int mosquitto_will_set_with_properties(struct mosquitto *mosq, const char *topic, int payloadlen, const void *payload, int qos, bool retain, mosquitto_property *properties) { + int rc; + if(!mosq) return MOSQ_ERR_INVAL; + + if(properties){ + rc = mosquitto_property_check_all(CMD_WILL, properties); + if(rc) return rc; + } + return will__set(mosq, topic, payloadlen, payload, qos, retain, properties); } diff --git a/lib/property_mosq.c b/lib/property_mosq.c index be8ef05e..e932d960 100644 --- a/lib/property_mosq.c +++ b/lib/property_mosq.c @@ -26,8 +26,6 @@ Contributors: #include "packet_mosq.h" #include "property_mosq.h" -static int property__command_check(int command, struct mqtt5__property *properties); - int property__read(struct mosquitto__packet *packet, int32_t *len, struct mqtt5__property *property) { @@ -146,7 +144,6 @@ int property__read_all(int command, struct mosquitto__packet *packet, struct mqt int rc; int32_t proplen; struct mqtt5__property *p, *tail = NULL; - struct mqtt5__property *current; rc = packet__read_varint(packet, &proplen, NULL); if(rc) return rc; @@ -172,54 +169,12 @@ int property__read_all(int command, struct mosquitto__packet *packet, struct mqt } tail = p; - /* Validity checks */ - if(p->identifier == MQTT_PROP_REQUEST_PROBLEM_INFORMATION - || p->identifier == MQTT_PROP_REQUEST_RESPONSE_INFORMATION - || p->identifier == MQTT_PROP_MAXIMUM_QOS - || p->identifier == MQTT_PROP_RETAIN_AVAILABLE - || p->identifier == MQTT_PROP_WILDCARD_SUB_AVAILABLE - || p->identifier == MQTT_PROP_SUBSCRIPTION_ID_AVAILABLE - || p->identifier == MQTT_PROP_SHARED_SUB_AVAILABLE){ - - if(p->value.i8 > 1){ - mosquitto_property_free_all(properties); - return MOSQ_ERR_PROTOCOL; - } - }else if(p->identifier == MQTT_PROP_MAXIMUM_PACKET_SIZE){ - if( p->value.i32 == 0){ - mosquitto_property_free_all(properties); - return MOSQ_ERR_PROTOCOL; - } - }else if(p->identifier == MQTT_PROP_RECEIVE_MAXIMUM - || p->identifier == MQTT_PROP_TOPIC_ALIAS){ - - if(p->value.i16 == 0){ - mosquitto_property_free_all(properties); - return MOSQ_ERR_PROTOCOL; - } - } } - /* Check for duplicates */ - current = *properties; - while(current){ - tail = current->next; - while(tail){ - if(current->identifier == tail->identifier - && current->identifier != MQTT_PROP_USER_PROPERTY){ - - mosquitto_property_free_all(properties); - return MOSQ_ERR_PROTOCOL; - } - tail = tail->next; - } - current = current->next; - } - - /* Check for properties on incorrect commands */ - if(property__command_check(command, *properties)){ + rc = mosquitto_property_check_all(command, *properties); + if(rc){ mosquitto_property_free_all(properties); - return MOSQ_ERR_PROTOCOL; + return rc; } return MOSQ_ERR_SUCCESS; } @@ -280,6 +235,8 @@ void mosquitto_property_free_all(struct mqtt5__property **property) { struct mqtt5__property *p, *next; + if(!property) return; + p = *property; while(p){ next = p->next; @@ -459,7 +416,7 @@ int property__write_all(struct mosquitto__packet *packet, const struct mqtt5__pr } -int mosquitto_property_command_check(int command, int identifier) +int mosquitto_property_check_command(int command, int identifier) { switch(identifier){ case MQTT_PROP_PAYLOAD_FORMAT_INDICATOR: @@ -552,21 +509,6 @@ int mosquitto_property_command_check(int command, int identifier) return MOSQ_ERR_SUCCESS; } -static int property__command_check(int command, struct mqtt5__property *properties) -{ - struct mqtt5__property *p; - int rc; - - p = properties; - while(p){ - rc = mosquitto_property_command_check(command, p->identifier); - if(rc) return rc; - - p = p->next; - } - return MOSQ_ERR_SUCCESS; -} - int mosquitto_string_to_property_info(const char *propname, int *identifier, int *type) { @@ -878,3 +820,56 @@ int mosquitto_property_add_string_pair(mosquitto_property **proplist, int identi property__add(proplist, prop); return MOSQ_ERR_SUCCESS; } + +int mosquitto_property_check_all(int command, const mosquitto_property *properties) +{ + const mosquitto_property *p, *tail; + int rc; + + p = properties; + + while(p){ + /* Validity checks */ + if(p->identifier == MQTT_PROP_REQUEST_PROBLEM_INFORMATION + || p->identifier == MQTT_PROP_REQUEST_RESPONSE_INFORMATION + || p->identifier == MQTT_PROP_MAXIMUM_QOS + || p->identifier == MQTT_PROP_RETAIN_AVAILABLE + || p->identifier == MQTT_PROP_WILDCARD_SUB_AVAILABLE + || p->identifier == MQTT_PROP_SUBSCRIPTION_ID_AVAILABLE + || p->identifier == MQTT_PROP_SHARED_SUB_AVAILABLE){ + + if(p->value.i8 > 1){ + return MOSQ_ERR_PROTOCOL; + } + }else if(p->identifier == MQTT_PROP_MAXIMUM_PACKET_SIZE){ + if( p->value.i32 == 0){ + return MOSQ_ERR_PROTOCOL; + } + }else if(p->identifier == MQTT_PROP_RECEIVE_MAXIMUM + || p->identifier == MQTT_PROP_TOPIC_ALIAS){ + + if(p->value.i16 == 0){ + return MOSQ_ERR_PROTOCOL; + } + } + + /* Check for properties on incorrect commands */ + rc = mosquitto_property_check_command(command, p->identifier); + if(rc) return rc; + + /* Check for duplicates */ + tail = p->next; + while(tail){ + if(p->identifier == tail->identifier + && p->identifier != MQTT_PROP_USER_PROPERTY){ + + return MOSQ_ERR_DUPLICATE_PROPERTY; + } + tail = tail->next; + } + + p = p->next; + } + + return MOSQ_ERR_SUCCESS; +} diff --git a/lib/will_mosq.c b/lib/will_mosq.c index b9a13b9d..154badbc 100644 --- a/lib/will_mosq.c +++ b/lib/will_mosq.c @@ -53,7 +53,7 @@ int will__set(struct mosquitto *mosq, const char *topic, int payloadlen, const v } p = properties; while(p){ - rc = mosquitto_property_command_check(CMD_WILL, p->identifier); + rc = mosquitto_property_check_command(CMD_WILL, p->identifier); if(rc) return rc; p = p->next; }