From patchwork Wed Jun 16 20:34:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1493051 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G4xkt0Vx8z9sW6 for ; Thu, 17 Jun 2021 06:35:01 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id AAADE404E1; Wed, 16 Jun 2021 20:34:59 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eIwLGYgmnzie; Wed, 16 Jun 2021 20:34:58 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id E7371401FF; Wed, 16 Jun 2021 20:34:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A40F9C000E; Wed, 16 Jun 2021 20:34:57 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 22B83C000B for ; Wed, 16 Jun 2021 20:34:56 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 11C974021B for ; Wed, 16 Jun 2021 20:34:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CY-sEt1gxLcB for ; Wed, 16 Jun 2021 20:34:55 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by smtp2.osuosl.org (Postfix) with ESMTPS id EAB0A401FF for ; Wed, 16 Jun 2021 20:34:54 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 81EDF100006; Wed, 16 Jun 2021 20:34:51 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Wed, 16 Jun 2021 22:34:48 +0200 Message-Id: <20210616203448.2324124-1-i.maximets@ovn.org> X-Mailer: git-send-email 2.26.3 MIME-Version: 1.0 Cc: Ilya Maximets Subject: [ovs-dev] [PATCH] ofp-actions: Report an error if there are too many actions to parse. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Not a very important fix, but fuzzer times out trying to test parsing of a huge number of actions. Fixing that by reporting an error as soon as ofpacts oversized. It would be great to use ofpacts_oversized() function instead of manual size checking, but ofpacts->header here always points to the last pushed action, so the value that ofpacts_oversized() would check is always small. Adding a unit test for this, plus the extra test for too deep nesting. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20254 Signed-off-by: Ilya Maximets Acked-by: Alin-Gabriel Serdean --- lib/ofp-actions.c | 4 ++++ tests/ofp-actions.at | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 6fb3da507..ecf914eac 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -9326,6 +9326,7 @@ static char * OVS_WARN_UNUSED_RESULT ofpacts_parse__(char *str, const struct ofpact_parse_params *pp, bool allow_instructions, enum ofpact_type outer_action) { + uint32_t orig_size = pp->ofpacts->size; char *key, *value; bool drop = false; char *pos; @@ -9370,6 +9371,9 @@ ofpacts_parse__(char *str, const struct ofpact_parse_params *pp, if (error) { return error; } + if (pp->ofpacts->size - orig_size > UINT16_MAX) { + return xasprintf("input too big"); + } } if (drop && pp->ofpacts->size) { diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 59093c03c..ef4898abb 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -1144,4 +1144,10 @@ bad_action 'apply_actions' 'apply_actions is the default instruction' bad_action 'xyzzy' 'unknown action xyzzy' bad_action 'drop,3' '"drop" must not be accompanied by any other action or instruction' +# Too many actions +writes=$(printf 'write_actions(%.0s' $(seq 100)) +bad_action "${writes}" 'Action nested too deeply' +outputs=$(printf '1,%.0s' $(seq 4096)) +bad_action "${outputs}" 'input too big' + AT_CLEANUP