Merge pull request #86 from volkszaehler/libFuzzer_fixes

libFuzzer fixes

sml/src/sml_boolean.c

@@ -41,6 +41,10 @@ sml_boolean *sml_boolean_parse(sml_buffer *buf) {
 		buf->error = 1;
 		return NULL;
 	}
+	if (buf->cursor >= buf->buffer_len) {
+		buf->error = 1;
+		return NULL;
+	}
 
 	if (sml_buf_get_current_byte(buf)) {
 		sml_buf_update_bytes_read(buf, 1);

sml/src/sml_get_profile_pack_request.c

@@ -118,8 +118,12 @@ sml_get_profile_pack_request *sml_get_profile_pack_request_parse(sml_buffer *buf
 			n = (sml_obj_req_entry_list *)malloc(sizeof(sml_obj_req_entry_list));
 			*n = (sml_obj_req_entry_list){.object_list_entry = NULL, .next = NULL};
 			n->object_list_entry = sml_obj_req_entry_parse(buf);
-			if (sml_buf_has_errors(buf))
+			if (sml_buf_has_errors(buf)) {
+				if (n->object_list_entry)
+					sml_octet_string_free(n->object_list_entry);
+				free(n);
 				goto error;
+			}
 
 			if (msg->object_list == 0) {
 				msg->object_list = n;

sml/src/sml_list.c

@@ -121,6 +121,7 @@ sml_list *sml_list_entry_parse(sml_buffer *buf, struct workarounds *workarounds)
 	static const unsigned char dzg_serial_start[] = {0x0a, 0x01, 'D', 'Z', 'G', 0x00};
 	static const unsigned char dzg_power_name[] = {1, 0, 16, 7, 0, 255};
 	u8 value_tl, value_len_more;
+	sml_list *l = NULL;
 
 	if (sml_buf_get_next_type(buf) != SML_TYPE_LIST) {
 		buf->error = 1;
@@ -131,7 +132,7 @@ sml_list *sml_list_entry_parse(sml_buffer *buf, struct workarounds *workarounds)
 		buf->error = 1;
 		goto error;
 	}
-	sml_list *l = sml_list_init();
+	l = sml_list_init();
 
 	l->obj_name = sml_octet_string_parse(buf);
 	if (sml_buf_has_errors(buf))
@@ -153,6 +154,10 @@ sml_list *sml_list_entry_parse(sml_buffer *buf, struct workarounds *workarounds)
 	if (sml_buf_has_errors(buf))
 		goto error;
 
+	if (buf->cursor >= buf->buffer_len) {
+		goto error;
+	}
+
 	value_tl = sml_buf_get_current_byte(buf);
 	value_len_more = value_tl & (SML_ANOTHER_TL | SML_LENGTH_FIELD);
 	l->value = sml_value_parse(buf);
@@ -202,10 +207,11 @@ sml_list *sml_list_entry_parse(sml_buffer *buf, struct workarounds *workarounds)
 
 	return l;
 
-// This function doesn't free the allocated memory in error cases,
-// this is done in sml_list_parse.
 error:
 	buf->error = 1;
+	if (l) {
+		sml_list_free(l);
+	}
 	return NULL;
 }
 

sml/src/sml_message.c

@@ -66,16 +66,29 @@ sml_message *sml_message_parse(sml_buffer *buf) {
 		goto error;
 
 	len = buf->cursor - msg_start;
+	if ((buf->buffer_len - buf->cursor) < 3) {
+		// libFuzzer ASAN found read out of bound.
+		// at least the 2 bytes for the crc and 1 for the sml_message_end there?
+		buf->error = 1;
+		goto error;
+	}
 
 	msg->crc = sml_u16_parse(buf);
-	if (sml_buf_has_errors(buf) || !(msg->crc))
+	if (sml_buf_has_errors(buf) || !(msg->crc)) {
+		buf->error = 1;
 		goto error;
+	}
 
 	if (*msg->crc != sml_crc16_calculate(&(buf->buffer[msg_start]), len))
 		// Workaround for Holley DTZ541 uses CRC-16/Kermit
 		if (*msg->crc != sml_crc16kermit_calculate(&(buf->buffer[msg_start]), len))
 			goto error;
 
+	if (buf->cursor >= buf->buffer_len) {
+		buf->error = 1;
+		goto error;
+	}
+
 	if (sml_buf_get_current_byte(buf) == SML_MESSAGE_END) {
 		sml_buf_update_bytes_read(buf, 1);
 	}
@@ -133,6 +146,11 @@ sml_message_body *sml_message_body_parse(sml_buffer *buf) {
 	sml_message_body *msg_body = (sml_message_body *)malloc(sizeof(sml_message_body));
 	*msg_body = (sml_message_body){.tag = NULL, .data = NULL};
 
+	if ((buf->cursor + 1) > buf->buffer_len) {
+		buf->error = 1;
+		goto error;
+	}
+
 	if (sml_buf_get_next_type(buf) != SML_TYPE_LIST) {
 		buf->error = 1;
 		goto error;
@@ -144,7 +162,7 @@ sml_message_body *sml_message_body_parse(sml_buffer *buf) {
 	}
 
 	msg_body->tag = sml_u32_parse(buf);
-	if (sml_buf_has_errors(buf))
+	if (sml_buf_has_errors(buf) || msg_body->tag == NULL)
 		goto error;
 
 	switch (*(msg_body->tag)) {

sml/src/sml_number.c

@@ -74,6 +74,11 @@ void *sml_number_parse(sml_buffer *buf, unsigned char type, int max_size) {
 	unsigned char b;
 	short negative_int = 0;
 
+	if ((buf->cursor + 1) > buf->buffer_len) { // at least 1 byte for type?
+		buf->error = 1;
+		return NULL;
+	}
+
 	if (sml_buf_get_next_type(buf) != type) {
 		buf->error = 1;
 		return NULL;
@@ -88,6 +93,19 @@ void *sml_number_parse(sml_buffer *buf, unsigned char type, int max_size) {
 	unsigned char *np = malloc(max_size);
 	memset(np, 0, max_size);
 
+	// at least l bytes available?
+	if ((buf->cursor + l) > buf->buffer_len) {
+		buf->error = 1;
+		free(np);
+		return NULL;
+	}
+
+	if ((buf->cursor + 1) > buf->buffer_len) { // at least 1 byte?
+		buf->error = 1;
+		free(np);
+		return NULL;
+	}
+
 	b = sml_buf_get_current_byte(buf);
 	if (type == SML_TYPE_INTEGER && (b & 128)) {
 		negative_int = 1;

sml/src/sml_octet_string.c

@@ -77,7 +77,8 @@ octet_string *sml_octet_string_parse(sml_buffer *buf) {
 	}
 
 	l = sml_buf_get_next_length(buf);
-	if (l < 0) {
+	if (l < 0 || l >= (buf->buffer_len - buf->cursor)) { // sanity check: doesnt fit into buffer...
+		// libFuzzer fix for crash-3b12f21fdd346700ac1a10dfebba7604be8f070c
 		buf->error = 1;
 		return NULL;
 	}

sml/src/sml_shared.c

@@ -22,6 +22,12 @@
 
 int sml_buf_get_next_length(sml_buffer *buf) {
 	int length = 0;
+
+	// current byte available?
+	if ((buf->cursor + 1) > buf->buffer_len) {
+		buf->error = 1;
+		return -1;
+	}
 	unsigned char byte = sml_buf_get_current_byte(buf);
 	int list = ((byte & SML_TYPE_FIELD) == SML_TYPE_LIST) ? 0 : -1;
 
@@ -38,7 +44,12 @@ int sml_buf_get_next_length(sml_buffer *buf) {
 			list += -1;
 		}
 	}
-	sml_buf_update_bytes_read(buf, 1);
+	if (buf->cursor < buf->buffer_len) {
+		sml_buf_update_bytes_read(buf, 1);
+	} else {
+		buf->error = 1;
+		return -1;
+	}
 
 	return length + list;
 }
@@ -91,7 +102,13 @@ void sml_buf_set_type_and_length(sml_buffer *buf, unsigned int type, unsigned in
 
 int sml_buf_has_errors(sml_buffer *buf) { return buf->error != 0; }
 
-int sml_buf_get_next_type(sml_buffer *buf) { return (buf->buffer[buf->cursor] & SML_TYPE_FIELD); }
+int sml_buf_get_next_type(sml_buffer *buf) {
+	if (buf->cursor >= buf->buffer_len) {
+		buf->error = 1;
+		return 0x100; // invalid type.
+	} else
+		return (buf->buffer[buf->cursor] & SML_TYPE_FIELD);
+}
 
 unsigned char sml_buf_get_current_byte(sml_buffer *buf) { return buf->buffer[buf->cursor]; }
 
@@ -126,7 +143,17 @@ void sml_buffer_free(sml_buffer *buf) {
 }
 
 int sml_buf_optional_is_skipped(sml_buffer *buf) {
+
+	if (buf->cursor >= buf->buffer_len) {
+		buf->error = 1;
+		return -1;
+	}
+
 	if (sml_buf_get_current_byte(buf) == SML_OPTIONAL_SKIPPED) {
+		if (buf->cursor + 1 > buf->buffer_len) {
+			buf->error = 1;
+			return -1;
+		}
 		sml_buf_update_bytes_read(buf, 1);
 
 		return 1;

sml/src/sml_time.c

@@ -72,18 +72,39 @@ sml_time *sml_time_parse(sml_buffer *buf) {
 		// Ignoring these values, so that parsing does not fail.
 		sml_buf_get_next_length(buf); // should we check the length here?
 		u32 *t1 = sml_u32_parse(buf);
-		if (sml_buf_has_errors(buf))
+		if (sml_buf_has_errors(buf)) {
+			if (t1)
+				sml_number_free(t1);
 			goto error;
+		}
 		i16 *t2 = sml_i16_parse(buf);
-		if (sml_buf_has_errors(buf))
+		if (sml_buf_has_errors(buf)) {
+			if (t1)
+				sml_number_free(t1);
+			if (t2)
+				sml_number_free(t2);
 			goto error;
+		}
 		i16 *t3 = sml_i16_parse(buf);
-		if (sml_buf_has_errors(buf))
+		if (sml_buf_has_errors(buf)) {
+			if (t1)
+				sml_number_free(t1);
+			if (t2)
+				sml_number_free(t2);
+			if (t3)
+				sml_number_free(t3);
 			goto error;
+		}
 		fprintf(
 			stderr,
 			"libsml: error: sml_time as list[3]: ignoring value[0]=%u value[1]=%d value[2]=%d\n",
-			*t1, *t2, *t3);
+			t1 ? *t1 : 0, t2 ? *t2 : 0, t3 ? *t3 : 0);
+		if (t1)
+			sml_number_free(t1);
+		if (t2)
+			sml_number_free(t2);
+		if (t3)
+			sml_number_free(t3);
 		break;
 	default:
 		goto error;

sml/src/sml_tree.c

@@ -37,8 +37,7 @@ sml_tree_path *sml_tree_path_parse(sml_buffer *buf) {
 	sml_tree_path *tree_path = sml_tree_path_init();
 
 	if (sml_buf_get_next_type(buf) != SML_TYPE_LIST) {
-		buf->error = 1;
-		return NULL;
+		goto error;
 	}
 
 	octet_string *s;
@@ -144,8 +143,11 @@ sml_tree *sml_tree_parse(sml_buffer *buf) {
 		int elems;
 		for (elems = sml_buf_get_next_length(buf); elems > 0; elems--) {
 			c = sml_tree_parse(buf);
-			if (sml_buf_has_errors(buf))
+			if (sml_buf_has_errors(buf)) {
+				if (c)
+					sml_tree_free(c);
 				goto error;
+			}
 			if (c) {
 				sml_tree_add_tree(tree, c);
 			}
@@ -229,7 +231,7 @@ sml_proc_par_value *sml_proc_par_value_parse(sml_buffer *buf) {
 	}
 
 	ppv->tag = sml_u8_parse(buf);
-	if (sml_buf_has_errors(buf))
+	if (sml_buf_has_errors(buf) || !(ppv->tag))
 		goto error;
 
 	switch (*(ppv->tag)) {

test/src/fuzzer.c

@@ -0,0 +1,40 @@
+// Copyright 2020 Matthias Behr
+//
+// This file is part of libSML.
+//
+// libSML is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// libSML is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with libSML.  If not, see <http://www.gnu.org/licenses/>.
+
+#include <sml/sml_file.h>
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+	// we shift by 8 to be able to use libsml-testing bin files inside
+	// corpus dir
+	sml_file *file = sml_file_parse(size > 8 ? ((unsigned char *)data) + 8 : (unsigned char *)data,
+									size > 8 ? (size - 8) : size);
+	sml_file_free(file);
+	return 0;
+}
+
+// compile with:
+// clang -g -O1 -L./sml/lib -fsanitize=fuzzer,address,undefined test/src/fuzzer.c `find sml/src/*.c`
+//  -luuid -I ./sml/include
+
+// run with:
+// mkdir corpus_dir
+// prefill corpus_dir e.g. with files libsml-testing
+// ./a.out corpus_dir
+// run until it stops with a crash file being generated
+// e.g. crash-...
+// debug with e.g.
+// gdb --args ./a.out crash-...