From e6cbff0e94ca6e39cd54dd88142d263261a37cba Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Sat, 24 Feb 2018 22:09:19 +0000 Subject: [PATCH] Fix unauthorised clients being able to cause OOM on connect. --- ChangeLog.txt | 5 +++++ lib/memory_mosq.c | 39 +++++++++++++++++++++++++++++++++++++++ lib/memory_mosq.h | 4 ++++ lib/net_mosq.c | 30 ++++++++++++++++++++++++++++++ src/conf.c | 8 ++++++++ 5 files changed, 86 insertions(+) diff --git a/ChangeLog.txt b/ChangeLog.txt index 96ec3db8..b6cd2542 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -2,6 +2,11 @@ Security: - Fix CVE-xxxx-xxxx. If a SIGHUP is sent to the broker when there are no more file descriptors, then opening the configuration file will fail and security settings will be set back to their default values. +- Fix CVE-xxxx-xxxx. Unauthenticated clients can cause excessive memory use by + setting "remaining length" to be a large value. This is now mitigated by + limiting the size of remaining length to valid values. A "memory_limit" + configuration option has also been added to allow the overall memory used by + the broker to be limited. Broker: - Use constant time memcmp for password comparisons. diff --git a/lib/memory_mosq.c b/lib/memory_mosq.c index dd3c50d0..8094ba03 100644 --- a/lib/memory_mosq.c +++ b/lib/memory_mosq.c @@ -37,8 +37,32 @@ static unsigned long memcount = 0; static unsigned long max_memcount = 0; #endif +#ifdef WITH_BROKER +static size_t mem_limit = 0; +void memory__set_limit(size_t lim) +{ +#ifdef LINUX + struct rlimit r; + + r.rlim_cur = lim; + r.rlim_max = lim; + + setrlimit(RLIMIT_CPU, &r); + + mem_limit = 0; +#else + mem_limit = lim; +#endif +} +#endif + void *_mosquitto_calloc(size_t nmemb, size_t size) { +#ifdef REAL_WITH_MEMORY_TRACKING + if(mem_limit && memcount + size > mem_limit){ + return NULL; + } +#endif void *mem = calloc(nmemb, size); #ifdef REAL_WITH_MEMORY_TRACKING @@ -64,6 +88,11 @@ void _mosquitto_free(void *mem) void *_mosquitto_malloc(size_t size) { +#ifdef REAL_WITH_MEMORY_TRACKING + if(mem_limit && memcount + size > mem_limit){ + return NULL; + } +#endif void *mem = malloc(size); #ifdef REAL_WITH_MEMORY_TRACKING @@ -90,6 +119,11 @@ unsigned long _mosquitto_max_memory_used(void) void *_mosquitto_realloc(void *ptr, size_t size) { +#ifdef REAL_WITH_MEMORY_TRACKING + if(mem_limit && memcount + size > mem_limit){ + return NULL; + } +#endif void *mem; #ifdef REAL_WITH_MEMORY_TRACKING if(ptr){ @@ -110,6 +144,11 @@ void *_mosquitto_realloc(void *ptr, size_t size) char *_mosquitto_strdup(const char *s) { +#ifdef REAL_WITH_MEMORY_TRACKING + if(mem_limit && memcount + strlen(s) > mem_limit){ + return NULL; + } +#endif char *str = strdup(s); #ifdef REAL_WITH_MEMORY_TRACKING diff --git a/lib/memory_mosq.h b/lib/memory_mosq.h index 6e14d7f4..0b8d71dd 100644 --- a/lib/memory_mosq.h +++ b/lib/memory_mosq.h @@ -34,4 +34,8 @@ unsigned long _mosquitto_max_memory_used(void); void *_mosquitto_realloc(void *ptr, size_t size); char *_mosquitto_strdup(const char *s); +#ifdef WITH_BROKER +void memory__set_limit(size_t lim); +#endif + #endif diff --git a/lib/net_mosq.c b/lib/net_mosq.c index 1aee1dd0..dc349695 100644 --- a/lib/net_mosq.c +++ b/lib/net_mosq.c @@ -1090,6 +1090,36 @@ int _mosquitto_packet_read(struct mosquitto *mosq) * positive. */ mosq->in_packet.remaining_count *= -1; +#ifdef WITH_BROKER + /* Check packet sizes before allocating memory. + * Will need modifying for MQTT v5. */ + switch(mosq->in_packet.command & 0xF0){ + case CONNECT: + if(mosq->in_packet.remaining_length > 327699){ + return MOSQ_ERR_PROTOCOL; + } + break; + + case PUBACK: + case PUBREC: + case PUBREL: + case PUBCOMP: + case UNSUBACK: + if(mosq->in_packet.remaining_length != 2){ + return MOSQ_ERR_PROTOCOL; + } + break; + + case PINGREQ: + case PINGRESP: + case DISCONNECT: + if(mosq->in_packet.remaining_length != 0){ + return MOSQ_ERR_PROTOCOL; + } + break; + } +#endif + if(mosq->in_packet.remaining_length > 0){ mosq->in_packet.payload = _mosquitto_malloc(mosq->in_packet.remaining_length*sizeof(uint8_t)); if(!mosq->in_packet.payload) return MOSQ_ERR_NOMEM; diff --git a/src/conf.c b/src/conf.c index c40232e0..19941736 100644 --- a/src/conf.c +++ b/src/conf.c @@ -1381,6 +1381,14 @@ int _config_read_file_core(struct mqtt3_config *config, bool reload, const char }else{ _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Empty max_queued_messages value in configuration."); } + }else if(!strcmp(token, "memory_limit")){ + size_t lim; + if(_conf_parse_int(&token, "memory_limit", (int *)&lim, saveptr)) return MOSQ_ERR_INVAL; + if(lim < 0){ + _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Invalid memory_limit value (%lu).", lim); + return MOSQ_ERR_INVAL; + } + memory__set_limit(lim); }else if(!strcmp(token, "message_size_limit")){ if(_conf_parse_int(&token, "message_size_limit", (int *)&config->message_size_limit, saveptr)) return MOSQ_ERR_INVAL; if(config->message_size_limit > MQTT_MAX_PAYLOAD){