From 03da4790a443ddd7f7c5acb4ecca8af419c4e5be Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sun, 2 Aug 2009 09:17:30 +0200 Subject: [PATCH] Improve buffer overflow check. With this patch smatch can now detect the buffer overflow from the lwn article about parfait. It has a lot of false positives though because it isn't always getting the array sizes correct. There are some new helper functions introduced in smatch.h. Signed-off-by: Dan Carpenter --- check_overflow.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++------- smatch.h | 5 +++ smatch_extra.c | 27 ++++++++++++++-- smatch_helper.c | 25 +++++++++++++++ 4 files changed, 139 insertions(+), 14 deletions(-) diff --git a/check_overflow.c b/check_overflow.c index a9f6ecf3..cfc1b345 100644 --- a/check_overflow.c +++ b/check_overflow.c @@ -10,15 +10,33 @@ #include #include "parse.h" #include "smatch.h" +#include "smatch_slist.h" static int my_id; -static struct smatch_state *alloc_state(int val) +static char *alloc_num(long long num) +{ + static char buff[256]; + + if (num == whole_range.min) { + snprintf(buff, 255, "min"); + } else if (num == whole_range.max) { + snprintf(buff, 255, "max"); + } else if (num < 0) { + snprintf(buff, 255, "(%lld)", num); + } else { + snprintf(buff, 255, "%lld", num); + } + buff[255] = '\0'; + return alloc_sname(buff); +} + +static struct smatch_state *alloc_my_state(int val) { struct smatch_state *state; state = malloc(sizeof(*state)); - state->name = "value"; + state->name = alloc_num(val); state->data = malloc(sizeof(int)); *(int *)state->data = val; return state; @@ -32,16 +50,17 @@ static int malloc_size(struct expression *expr) if (!expr) return 0; + expr = strip_expr(expr); if (expr->type == EXPR_CALL) { name = get_variable_from_expr(expr->fn, NULL); if (name && !strcmp(name, "kmalloc")) { arg = get_argument_from_call_expr(expr->args, 0); free_string(name); - return get_value(arg); + return get_value(arg) * 8; } free_string(name); } else if (expr->type == EXPR_STRING && expr->string) { - return expr->string->length; + return expr->string->length * 8; } return 0; } @@ -59,15 +78,65 @@ static void match_declaration(struct symbol *sym) base_type = get_base_type(sym); if (base_type->type == SYM_ARRAY && base_type->bit_size > 0) { - set_state(name, my_id, NULL, alloc_state(base_type->bit_size / 8)); + set_state(name, my_id, NULL, alloc_my_state(base_type->bit_size)); } else { size = malloc_size(sym->initializer); if (size > 0) - set_state(name, my_id, NULL, alloc_state(size)); + set_state(name, my_id, NULL, alloc_my_state(size)); } } +static int get_array_size(struct expression *expr) +{ + char *name; + struct symbol *tmp; + struct smatch_state *state; + int ret = 0; + + if (expr->type != EXPR_SYMBOL) + return 0; + name = get_variable_from_expr(expr, NULL); + if (!name) + return 0; + state = get_state(name, my_id, NULL); + if (!state || !state->data) + goto free; + tmp = get_base_type(expr->symbol); + if (tmp->type == SYM_PTR) + tmp = get_base_type(tmp); + ret = *(int *)state->data / 8 / tmp->ctype.alignment; +free: + free_string(name); + return ret; +} + +static void array_check(struct expression *expr) +{ + struct expression *dest; + int array_size; + struct expression *offset; + int max; + char *name; + + expr = strip_expr(expr); + if (!is_array(expr)) + return; + + dest = get_array_name(expr); + array_size = get_array_size(dest); + if (!array_size) + return; + + offset = get_array_offset(expr); + max = get_implied_max(offset); + if (array_size <= max) { + name = get_variable_from_expr(dest, NULL); + smatch_msg("error: buffer overflow '%s' %d <= %d", name, array_size, max); + free_string(name); + } +} + static void match_assignment(struct expression *expr) { char *name; @@ -75,7 +144,7 @@ static void match_assignment(struct expression *expr) if (!name) return; if (malloc_size(expr->right) > 0) - set_state(name, my_id, NULL, alloc_state(malloc_size(expr->right))); + set_state(name, my_id, NULL, alloc_my_state(malloc_size(expr->right))); free_string(name); } @@ -88,6 +157,8 @@ static void match_strcpy(const char *fn, struct expression *expr, char *data_name = NULL; struct smatch_state *dest_state; struct smatch_state *data_state; + int dest_size; + int data_size; dest = get_argument_from_call_expr(expr->args, 0); dest_name = get_variable_from_expr(dest, NULL); @@ -102,11 +173,11 @@ static void match_strcpy(const char *fn, struct expression *expr, data_state = get_state(data_name, my_id, NULL); if (!data_state || !data_state->data) goto free; - - if (*(int *)dest_state->data < *(int *)data_state->data) + dest_size = *(int *)dest_state->data / 8; + data_size = *(int *)data_state->data / 8; + if (dest_size < data_size) smatch_msg("error: %s (%d) too large for %s (%d)", data_name, - *(int *)data_state->data, - dest_name, *(int *)dest_state->data); + data_size, dest_name, dest_size); free: free_string(dest_name); free_string(data_name); @@ -130,7 +201,7 @@ static void match_limitted(const char *fn, struct expression *expr, state = get_state(dest_name, my_id, NULL); if (!state || !state->data) goto free; - has = *(int *)state->data; + has = *(int *)state->data / 8; if (has < needed) smatch_msg("error: %s too small for %d bytes.", dest_name, needed); @@ -142,6 +213,7 @@ void check_overflow(int id) { my_id = id; add_hook(&match_declaration, DECLARATION_HOOK); + add_hook(&array_check, OP_HOOK); add_hook(&match_assignment, ASSIGNMENT_HOOK); add_function_hook("strcpy", &match_strcpy, NULL); add_function_hook("strncpy", &match_limitted, (void *)2); diff --git a/smatch.h b/smatch.h index a84a9e6d..ebcc1e1d 100644 --- a/smatch.h +++ b/smatch.h @@ -159,6 +159,9 @@ struct symbol *get_ptr_type(struct expression *expr); int sym_name_is(const char *name, struct expression *expr); int get_value(struct expression *expr); int is_zero(struct expression *expr); +int is_array(struct expression *expr); +struct expression *get_array_name(struct expression *expr); +struct expression *get_array_offset(struct expression *expr); const char *show_state(struct smatch_state *state); struct statement *get_block_thing(struct expression *expr); struct expression *strip_expr(struct expression *expr); @@ -221,6 +224,8 @@ struct data_range { extern struct data_range whole_range; int get_implied_value(struct expression *expr); +int get_implied_max(struct expression *expr); +int get_implied_min(struct expression *expr); int true_comparison(int left, int comparison, int right); int known_condition_true(struct expression *expr); int known_condition_false(struct expression *expr); diff --git a/smatch_extra.c b/smatch_extra.c index 9536872a..24380312 100644 --- a/smatch_extra.c +++ b/smatch_extra.c @@ -332,7 +332,11 @@ static void match_function_def(struct symbol *sym) } END_FOR_EACH_PTR(arg); } -int get_implied_value(struct expression *expr) +#define VAL_SINGLE 0 +#define VAL_MAX 1 +#define VAL_MIN 2 + +static int get_implied_value_helper(struct expression *expr, int what) { struct smatch_state *state; int val; @@ -350,7 +354,26 @@ int get_implied_value(struct expression *expr) free_string(name); if (!state || !state->data) return UNDEFINED; - return get_single_value_from_range((struct data_info *)state->data); + if (what == VAL_SINGLE) + return get_single_value_from_range((struct data_info *)state->data); + if (what == VAL_MAX) + return get_dinfo_max((struct data_info *)state->data); + return get_dinfo_min((struct data_info *)state->data); +} + +int get_implied_value(struct expression *expr) +{ + return get_implied_value_helper(expr, VAL_SINGLE); +} + +int get_implied_max(struct expression *expr) +{ + return get_implied_value_helper(expr, VAL_MAX); +} + +int get_implied_min(struct expression *expr) +{ + return get_implied_value_helper(expr, VAL_MIN); } int last_stmt_val(struct statement *stmt) diff --git a/smatch_helper.c b/smatch_helper.c index 2bf783c5..d34b3b75 100644 --- a/smatch_helper.c +++ b/smatch_helper.c @@ -423,6 +423,31 @@ int is_zero(struct expression *expr) return 0; } +int is_array(struct expression *expr) +{ + expr = strip_expr(expr); + if (expr->type != EXPR_PREOP || strcmp(show_special(expr->op), "*")) + return 0; + expr = expr->unop; + if (strcmp(show_special(expr->op), "+")) + return 0; + return 1; +} + +struct expression *get_array_name(struct expression *expr) +{ + if (!is_array(expr)) + return NULL; + return expr->unop->left; +} + +struct expression *get_array_offset(struct expression *expr) +{ + if (!is_array(expr)) + return NULL; + return expr->unop->right; +} + const char *show_state(struct smatch_state *state) { if (!state) -- 2.11.4.GIT