From 7784140f2bd2d5ae44f2be1507ac25f102006155 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Wed, 29 Apr 2020 17:04:10 -0400 Subject: misc: binary api fuzz test fixes Add a hook to src/vlibapi/api_shared.c to fuzz (screw up) binary API messages, e.g. by xoring random data into them before processing. We specifically exempt client connection messages, and inband debug CLI messages. We step over msg_id, client index, client context, and sw_if_index. Otherwise, "make test" vectors fail too rapidly to learn anything. The goal is to reduce the number of crashes caused to zero. We're fairly close with this patch. Add vl_msg_api_max_length(void *mp), which returns the maximum plausible length for a binary API message. Use it to hardern vl_api_from_api_to_new_vec(...) which takes an additional argument - message pointer - so it can verify that astr->length is sane. If it's not sane, return a u8 *vector of the form "insane astr->length nnnn\0". Verify array lengths in vl_api_dhcp6_send_client_message_t_handler(...) and vl_api_dhcp6_pd_send_client_message_t_handler(...). Add a fairly effective binary API fuzz hook to the unittest plugin, and modify the "make test" framework.py to pass "api-fuzz { on|off }" to enable API fuzzing: "make API_FUZZ=on TEST=xxx test-debug" or similar Type: improvement Signed-off-by: Dave Barach Change-Id: I0157267652a163c01553d5267620f719cc6c3bde --- src/plugins/dhcp/dhcp_api.c | 14 +++ src/plugins/ioam/lib-pot/pot_api.c | 10 +- src/plugins/unittest/CMakeLists.txt | 1 + src/plugins/unittest/api_fuzz_test.c | 187 +++++++++++++++++++++++++++++++++++ src/vat/api_format.c | 4 +- src/vlibapi/api.h | 1 + src/vlibapi/api_shared.c | 27 ++++- src/vlibapi/api_types.h | 2 +- src/vnet/devices/tap/tapv2_api.c | 2 +- src/vnet/interface_api.c | 2 +- src/vnet/ip/ip_api.c | 5 +- src/vnet/ip/punt_api.c | 2 +- src/vnet/lldp/lldp_api.c | 2 +- src/vnet/pg/pg_api.c | 2 +- src/vnet/session/session_api.c | 4 +- src/vpp/api/api.c | 2 +- src/vpp/api/custom_dump.c | 4 +- 17 files changed, 250 insertions(+), 21 deletions(-) create mode 100644 src/plugins/unittest/api_fuzz_test.c (limited to 'src') diff --git a/src/plugins/dhcp/dhcp_api.c b/src/plugins/dhcp/dhcp_api.c index a5163ca2e95..d2e423572fb 100644 --- a/src/plugins/dhcp/dhcp_api.c +++ b/src/plugins/dhcp/dhcp_api.c @@ -549,6 +549,12 @@ void params.T1 = ntohl (mp->T1); params.T2 = ntohl (mp->T2); n_addresses = ntohl (mp->n_addresses); + /* Make sure that the number of addresses is sane */ + if (n_addresses * sizeof (params.addresses) > vl_msg_api_max_length (mp)) + { + rv = VNET_API_ERROR_INVALID_VALUE; + goto bad_sw_if_index; + } params.addresses = 0; if (n_addresses > 0) vec_validate (params.addresses, n_addresses - 1); @@ -593,6 +599,14 @@ void params.T1 = ntohl (mp->T1); params.T2 = ntohl (mp->T2); n_prefixes = ntohl (mp->n_prefixes); + + /* Minimal check to see that the number of prefixes is sane */ + if (n_prefixes * sizeof (params.prefixes) > vl_msg_api_max_length (mp)) + { + rv = VNET_API_ERROR_INVALID_VALUE; + goto bad_sw_if_index; + } + params.prefixes = 0; if (n_prefixes > 0) vec_validate (params.prefixes, n_prefixes - 1); diff --git a/src/plugins/ioam/lib-pot/pot_api.c b/src/plugins/ioam/lib-pot/pot_api.c index 31ddf9da528..72d05d76590 100644 --- a/src/plugins/ioam/lib-pot/pot_api.c +++ b/src/plugins/ioam/lib-pot/pot_api.c @@ -14,7 +14,7 @@ */ /* *------------------------------------------------------------------ - * pot_api.c - Proof of Transit related APIs to create + * pot_api.c - Proof of Transit related APIs to create * and maintain profiles *------------------------------------------------------------------ */ @@ -43,7 +43,7 @@ static void vl_api_pot_profile_add_t_handler pot_profile *profile = NULL; u8 *name = 0; - name = vl_api_from_api_to_new_vec(&mp->list_name); + name = vl_api_from_api_to_new_vec(mp, &mp->list_name); pot_profile_list_init(name); id = mp->id; @@ -61,7 +61,7 @@ static void vl_api_pot_profile_add_t_handler (void)pot_profile_set_bit_mask(profile, mp->max_bits); } else { rv = -3; - } + } ERROROUT: vec_free(name); REPLY_MACRO(VL_API_POT_PROFILE_ADD_REPLY); @@ -121,14 +121,14 @@ static void vl_api_pot_profile_activate_t_handler u8 id; u8 *name = NULL; - name = vl_api_from_api_to_new_vec(&mp->list_name); + name = vl_api_from_api_to_new_vec(mp, &mp->list_name); if (!pot_profile_list_is_enabled(name)) { rv = -1; } else { id = mp->id; rv = pot_profile_set_active(id); } - + vec_free(name); REPLY_MACRO(VL_API_POT_PROFILE_ACTIVATE_REPLY); } diff --git a/src/plugins/unittest/CMakeLists.txt b/src/plugins/unittest/CMakeLists.txt index 7ab63dad6a4..3ea4752145b 100644 --- a/src/plugins/unittest/CMakeLists.txt +++ b/src/plugins/unittest/CMakeLists.txt @@ -13,6 +13,7 @@ add_vpp_plugin(unittest SOURCES + api_fuzz_test.c bier_test.c bihash_test.c bitmap_test.c diff --git a/src/plugins/unittest/api_fuzz_test.c b/src/plugins/unittest/api_fuzz_test.c new file mode 100644 index 00000000000..113835300bb --- /dev/null +++ b/src/plugins/unittest/api_fuzz_test.c @@ -0,0 +1,187 @@ +/* + *------------------------------------------------------------------ + * api_fuzz_test.c - Binary API fuzz hook + * + * Copyright (c) 2020 Cisco and/or its affiliates. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *------------------------------------------------------------------ + */ +#include +#include +#include +#include +#include +#include + +static u32 fuzz_seed = 0xdeaddabe; +static u16 fuzz_first; +static u16 fuzz_cli_first, fuzz_cli_last; + +extern void (*vl_msg_api_fuzz_hook) (u16, void *); + +static void +fuzz_hook (u16 id, void *the_msg) +{ + /* + * Fuzz (aka screw up) this message? Leave connection establishment + * messages alone as well as CLI messages. + */ + if ((id > fuzz_first) && !(id >= fuzz_cli_first && id < fuzz_cli_last)) + { + msgbuf_t *mb; + u8 *limit, *start; + + mb = (msgbuf_t *) (((u8 *) the_msg) - offsetof (msgbuf_t, data)); + + limit = (u8 *) (mb->data + ntohl (mb->data_len)); + + /* + * Leave the first 14 octets alone, aka msg_id, client_index, + * context, sw_if_index + */ + + start = ((u8 *) the_msg) + 14; + + for (; start < limit; start++) + *start ^= (random_u32 (&fuzz_seed) & 0xFF); + } +} + +static void +default_fuzz_config (void) +{ + fuzz_first = vl_msg_api_get_msg_index + ((u8 *) "memclnt_keepalive_reply_e8d4e804"); + fuzz_cli_first = vl_msg_api_get_msg_index ((u8 *) "cli_23bfbfff"); + fuzz_cli_last = vl_msg_api_get_msg_index + ((u8 *) "cli_inband_reply_05879051"); +} + +static clib_error_t * +test_api_fuzz_command_fn (vlib_main_t * vm, + unformat_input_t * input, vlib_cli_command_t * cmd) +{ + u32 tmp; + + default_fuzz_config (); + + if (fuzz_first == 0xFFFF) + { + vlib_cli_output (vm, "Couldn't find 'memclnt_keepalive_reply' ID"); + vlib_cli_output + (vm, "Manual setting required, use 'show api message table'"); + } + + if (fuzz_cli_first == 0xFFFF) + { + vlib_cli_output (vm, "Couldn't find 'cli' ID"); + vlib_cli_output + (vm, "Manual setting required, use 'show api message table'"); + } + + if (fuzz_cli_last == 0xFFFF) + { + vlib_cli_output (vm, "Couldn't find 'cli_inband_reply' ID"); + vlib_cli_output + (vm, "Manual setting required, use 'show api message table'"); + } + + while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT) + { + if (unformat (input, "seed %d", &fuzz_seed)) + ; + else if (unformat (input, "disable") | unformat (input, "off")) + fuzz_first = ~0; + else if (unformat (input, "fuzz-first %d", &tmp)) + fuzz_first = (u16) tmp; + else if (unformat (input, "fuzz-cli-first %d", &tmp)) + fuzz_cli_first = (u16) tmp; + else if (unformat (input, "fuzz-cli-last %d", &tmp)) + fuzz_cli_last = (u16) tmp; + else + break; + } + + if (fuzz_first == 0xFFFF) + { + vl_msg_api_fuzz_hook = 0; + return clib_error_return (0, "fuzz_first is ~0, fuzzing disabled"); + } + vl_msg_api_fuzz_hook = fuzz_hook; + + vlib_cli_output (vm, "Fuzzing enabled: first %d, skip cli range %d - %d", + (u32) fuzz_first, (u32) fuzz_cli_first, + (u32) fuzz_cli_last); + + return 0; +} + +/* *INDENT-OFF* */ +VLIB_CLI_COMMAND (test_api_fuzz, static) = { + .path = "test api fuzz", + .short_help = "test api fuzz [disable][seed nnn]\n" + " [fuzz-first nn][fuzz-cli-first nn][fuzz-cli-last nn]", + .function = test_api_fuzz_command_fn, + }; +/* *INDENT-ON* */ + +static u8 main_loop_enter_enable_api_fuzz; + +static clib_error_t * +api_fuzz_config (vlib_main_t * vm, unformat_input_t * input) +{ + while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT) + { + if (unformat (input, "off") + || unformat (input, "disable") || unformat (input, "no")) + ; /* ok, no action */ + else if (unformat (input, "on") + || unformat (input, "enable") || unformat (input, "yes")) + main_loop_enter_enable_api_fuzz = 1; + else + return clib_error_return (0, "unknown input '%U'", + format_unformat_error, input); + } + return 0; +} + +VLIB_CONFIG_FUNCTION (api_fuzz_config, "api-fuzz"); + +static clib_error_t * +api_fuzz_api_init (vlib_main_t * vm) +{ + /* Are we supposed to fuzz API messages? */ + if (main_loop_enter_enable_api_fuzz == 0) + return 0; + + default_fuzz_config (); + + if (fuzz_first == 0xFFFF) + { + return clib_error_return + (0, "Couldn't find 'memclnt_keepalive_reply' ID"); + } + /* Turn on fuzzing */ + vl_msg_api_fuzz_hook = fuzz_hook; + return 0; +} + +VLIB_API_INIT_FUNCTION (api_fuzz_api_init); + +/* + * fd.io coding-style-patch-verification: ON + * + * Local Variables: + * eval: (c-set-style "gnu") + * End: + */ diff --git a/src/vat/api_format.c b/src/vat/api_format.c index d443239a940..7d16441f03b 100644 --- a/src/vat/api_format.c +++ b/src/vat/api_format.c @@ -1165,7 +1165,7 @@ vl_api_cli_inband_reply_t_handler (vl_api_cli_inband_reply_t * mp) vam->retval = retval; if (retval == 0) - vam->cmd_reply = vl_api_from_api_to_new_vec (&mp->reply); + vam->cmd_reply = vl_api_from_api_to_new_vec (mp, &mp->reply); vam->result_ready = 1; } @@ -1176,7 +1176,7 @@ vl_api_cli_inband_reply_t_handler_json (vl_api_cli_inband_reply_t * mp) vat_json_node_t node; u8 *reply = 0; /* reply vector */ - reply = vl_api_from_api_to_new_vec (&mp->reply); + reply = vl_api_from_api_to_new_vec (mp, &mp->reply); vec_reset_length (vam->cmd_reply); vat_json_init_object (&node); diff --git a/src/vlibapi/api.h b/src/vlibapi/api.h index 3d5c8698f73..431155c5e09 100644 --- a/src/vlibapi/api.h +++ b/src/vlibapi/api.h @@ -109,6 +109,7 @@ void vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp, void *the_msg, vlib_main_t * vm, vlib_node_runtime_t * node, u8 is_private); +u32 vl_msg_api_max_length (void *mp); vl_api_trace_t *vl_msg_api_trace_get (api_main_t * am, vl_api_trace_which_t which); void vl_msg_api_add_msg_name_crc (api_main_t * am, const char *string, diff --git a/src/vlibapi/api_shared.c b/src/vlibapi/api_shared.c index 28e9f481650..fbaa9c9e013 100644 --- a/src/vlibapi/api_shared.c +++ b/src/vlibapi/api_shared.c @@ -533,6 +533,8 @@ msg_handler_internal (api_main_t * am, } } +void (*vl_msg_api_fuzz_hook) (u16, void *); + /* This is only to be called from a vlib/vnet app */ void vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp, @@ -600,6 +602,10 @@ vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp, am->vlib_rp = vlib_rp; am->shmem_hdr = (void *) vlib_rp->user_ctx; } + + if (PREDICT_FALSE (vl_msg_api_fuzz_hook != 0)) + (*vl_msg_api_fuzz_hook) (id, the_msg); + (*handler) (the_msg, vm, node); if (is_private) { @@ -870,6 +876,21 @@ vl_msg_api_queue_handler (svm_queue_t * q) vl_msg_api_handler ((void *) msg); } +u32 +vl_msg_api_max_length (void *mp) +{ + msgbuf_t *mb; + u32 data_len = ~0; + + /* Work out the maximum sane message length, and return it */ + if (PREDICT_TRUE (mp != 0)) + { + mb = (msgbuf_t *) (((u8 *) mp) - offsetof (msgbuf_t, data)); + data_len = clib_net_to_host_u32 (mb->data_len); + } + return data_len; +} + vl_api_trace_t * vl_msg_api_trace_get (api_main_t * am, vl_api_trace_which_t which) { @@ -1132,9 +1153,13 @@ vl_api_format_string (u8 * s, va_list * args) * NOT nul terminated. */ u8 * -vl_api_from_api_to_new_vec (vl_api_string_t * astr) +vl_api_from_api_to_new_vec (void *mp, vl_api_string_t * astr) { u8 *v = 0; + + if (vl_msg_api_max_length (mp) < clib_net_to_host_u32 (astr->length)) + return format (0, "insane astr->length %u%c", + clib_net_to_host_u32 (astr->length), 0); vec_add (v, astr->buf, clib_net_to_host_u32 (astr->length)); return v; } diff --git a/src/vlibapi/api_types.h b/src/vlibapi/api_types.h index 21fcde53b0e..07c7aaba0c5 100644 --- a/src/vlibapi/api_types.h +++ b/src/vlibapi/api_types.h @@ -46,7 +46,7 @@ extern int vl_api_vec_to_api_string (const u8 *vec, vl_api_string_t * str); extern u32 vl_api_string_len (vl_api_string_t * astr); /* Returns new vector. NON nul terminated */ -extern u8 * vl_api_from_api_to_new_vec (vl_api_string_t *astr); +extern u8 * vl_api_from_api_to_new_vec (void *mp, vl_api_string_t *astr); /* Returns new vector. Nul terminated */ extern char * vl_api_from_api_to_new_c_string (vl_api_string_t *astr); diff --git a/src/vnet/devices/tap/tapv2_api.c b/src/vnet/devices/tap/tapv2_api.c index 05679377d2a..5aca93ec310 100644 --- a/src/vnet/devices/tap/tapv2_api.c +++ b/src/vnet/devices/tap/tapv2_api.c @@ -134,7 +134,7 @@ vl_api_tap_create_v2_t_handler (vl_api_tap_create_v2_t * mp) /* If a tag was supplied... */ if (vl_api_string_len (&mp->tag)) { - u8 *tag = vl_api_from_api_to_new_vec (&mp->tag); + u8 *tag = vl_api_from_api_to_new_vec (mp, &mp->tag); vnet_set_sw_interface_tag (vnm, tag, ap->sw_if_index); } diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c index 5b24a29efc9..846420d7378 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -361,7 +361,7 @@ vl_api_sw_interface_dump_t_handler (vl_api_sw_interface_dump_t * mp) if (mp->name_filter_valid) { - filter = vl_api_from_api_to_new_vec (&mp->name_filter); + filter = vl_api_from_api_to_new_vec (mp, &mp->name_filter); vec_add1 (filter, 0); /* Ensure it's a C string for strcasecmp() */ } diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c index 3bc46fee59b..d5fa2d32f21 100644 --- a/src/vnet/ip/ip_api.c +++ b/src/vnet/ip/ip_api.c @@ -1544,13 +1544,14 @@ static void vl_api_ip_punt_redirect_dump_t_handler (vl_api_ip_punt_redirect_dump_t * mp) { vl_api_registration_t *reg; - fib_protocol_t fproto; + fib_protocol_t fproto = FIB_PROTOCOL_IP4; reg = vl_api_client_index_to_registration (mp->client_index); if (!reg) return; - fproto = mp->is_ipv6 ? FIB_PROTOCOL_IP6 : FIB_PROTOCOL_IP4; + if (mp->is_ipv6 == 1) + fproto = FIB_PROTOCOL_IP6; ip_punt_redirect_walk_ctx_t ctx = { .reg = reg, diff --git a/src/vnet/ip/punt_api.c b/src/vnet/ip/punt_api.c index 2acf8265dc1..3a964b4b7f5 100644 --- a/src/vnet/ip/punt_api.c +++ b/src/vnet/ip/punt_api.c @@ -372,7 +372,7 @@ vl_api_punt_reason_dump_t_handler (vl_api_punt_reason_dump_t * mp) punt_reason_dump_walk_ctx_t ctx = { .reg = reg, .context = mp->context, - .name = vl_api_from_api_to_new_vec (&mp->reason.name), + .name = vl_api_from_api_to_new_vec (mp, &mp->reason.name), }; punt_reason_walk (punt_reason_dump_walk_cb, &ctx); diff --git a/src/vnet/lldp/lldp_api.c b/src/vnet/lldp/lldp_api.c index a6d7fb84bb7..ecb75bcac4b 100644 --- a/src/vnet/lldp/lldp_api.c +++ b/src/vnet/lldp/lldp_api.c @@ -57,7 +57,7 @@ vl_api_lldp_config_t_handler (vl_api_lldp_config_t * mp) int rv = 0; u8 *sys_name = 0; - sys_name = vl_api_from_api_to_new_vec (&mp->system_name); + sys_name = vl_api_from_api_to_new_vec (mp, &mp->system_name); if (lldp_cfg_set (&sys_name, ntohl (mp->tx_hold), ntohl (mp->tx_interval)) != lldp_ok) diff --git a/src/vnet/pg/pg_api.c b/src/vnet/pg/pg_api.c index 18de1e9e8c4..bb58a4f0cec 100644 --- a/src/vnet/pg/pg_api.c +++ b/src/vnet/pg/pg_api.c @@ -122,7 +122,7 @@ vl_api_pg_enable_disable_t_handler (vl_api_pg_enable_disable_t * mp) if (vl_api_string_len (&mp->stream_name) > 0) { - u8 *stream_name = vl_api_from_api_to_new_vec (&mp->stream_name); + u8 *stream_name = vl_api_from_api_to_new_vec (mp, &mp->stream_name); uword *p = hash_get_mem (pg->stream_index_by_name, stream_name); if (p) stream_index = *p; diff --git a/src/vnet/session/session_api.c b/src/vnet/session/session_api.c index cfeb7bcc349..593f2e18ba0 100644 --- a/src/vnet/session/session_api.c +++ b/src/vnet/session/session_api.c @@ -624,7 +624,7 @@ vl_api_app_attach_t_handler (vl_api_app_attach_t * mp) a->options = mp->options; a->session_cb_vft = &session_mq_cb_vft; - a->namespace_id = vl_api_from_api_to_new_vec (&mp->namespace_id); + a->namespace_id = vl_api_from_api_to_new_vec (mp, &mp->namespace_id); if ((rv = vnet_application_attach (a))) { @@ -801,7 +801,7 @@ vl_api_app_namespace_add_del_t_handler (vl_api_app_namespace_add_del_t * mp) goto done; } - ns_id = vl_api_from_api_to_new_vec (&mp->namespace_id); + ns_id = vl_api_from_api_to_new_vec (mp, &mp->namespace_id); vnet_app_namespace_add_del_args_t args = { .ns_id = ns_id, diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c index 01527a565f3..c3380be0b80 100644 --- a/src/vpp/api/api.c +++ b/src/vpp/api/api.c @@ -221,7 +221,7 @@ vl_api_cli_inband_t_handler (vl_api_cli_inband_t * mp) goto error; } - cmd_vec = vl_api_from_api_to_new_vec (&mp->cmd); + cmd_vec = vl_api_from_api_to_new_vec (mp, &mp->cmd); unformat_init_string (&input, (char *) cmd_vec, vl_api_string_len (&mp->cmd)); diff --git a/src/vpp/api/custom_dump.c b/src/vpp/api/custom_dump.c index e0ba9f93c3d..edbd3b65e9d 100644 --- a/src/vpp/api/custom_dump.c +++ b/src/vpp/api/custom_dump.c @@ -1799,7 +1799,7 @@ static void *vl_api_sw_interface_dump_t_print if (mp->name_filter_valid) { - u8 *v = vl_api_from_api_to_new_vec (&mp->name_filter); + u8 *v = vl_api_from_api_to_new_vec (mp, &mp->name_filter); s = format (s, "name_filter %v ", v); vec_free (v); } @@ -1858,7 +1858,7 @@ static void *vl_api_cli_inband_t_print u8 *s; u8 *cmd = 0; - cmd = vl_api_from_api_to_new_vec (&mp->cmd); + cmd = vl_api_from_api_to_new_vec (mp, &mp->cmd); s = format (0, "SCRIPT: exec %v ", cmd); -- cgit 1.2.3-korg