From patchwork Fri Feb 2 07:29:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Dickinson X-Patchwork-Id: 868526 X-Patchwork-Delegate: blogic@openwrt.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (helo) smtp.helo=arrakis.dune.hu (client-ip=78.24.191.176; helo=arrakis.dune.hu; envelope-from=openwrt-devel-bounces@lists.openwrt.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=thecshore.com header.i=@thecshore.com header.b="TLLlf10I"; dkim-atps=neutral Received: from arrakis.dune.hu (arrakis.dune.hu [78.24.191.176]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zXpYB5MZgz9sPk for ; Fri, 2 Feb 2018 18:30:21 +1100 (AEDT) Received: from arrakis.dune.hu (localhost [127.0.0.1]) by arrakis.dune.hu (Postfix) with ESMTP id 61884B80D39; Fri, 2 Feb 2018 08:30:08 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on arrakis.dune.hu X-Spam-Level: X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_00,T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 Received: from arrakis.dune.hu (localhost [127.0.0.1]) by arrakis.dune.hu (Postfix) with ESMTP; Fri, 2 Feb 2018 08:30:08 +0100 (CET) Received: from arrakis.dune.hu (localhost [127.0.0.1]) by arrakis.dune.hu (Postfix) with ESMTP id D22E2B80632 for ; Fri, 2 Feb 2018 08:30:06 +0100 (CET) X-policyd-weight: using cached result; rate: -6.1 Received: from mail.thecshore.com (mail.thecshore.com [144.217.14.6]) by arrakis.dune.hu (Postfix) with ESMTP for ; Fri, 2 Feb 2018 08:30:06 +0100 (CET) Received: from workhobbyl.thecshore.com (135-23-247-100.cpe.pppoe.ca [135.23.247.100]) by mail.thecshore.com (Postfix) with ESMTPSA id C828C27B0; Fri, 2 Feb 2018 02:30:03 -0500 (EST) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.thecshore.com C828C27B0 Authentication-Results: mail.thecshore.com; dmarc=none (p=none dis=none) header.from=thecshore.com Authentication-Results: mail.thecshore.com; spf=fail smtp.mailfrom=cshored@thecshore.com DKIM-Filter: OpenDKIM Filter v2.11.0 mail.thecshore.com C828C27B0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=thecshore.com; s=default; t=1517556603; bh=ZL9dFNormLXEXElNLNykG+6fQEgNhipnvf2WBpwN2BI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TLLlf10IIxMRneruEQvjt3DoHfmpwRkI85LeHgzkogAiL0py35JkYdUdrRoeXjCng OD8l87T5sSUAWu9RYhWdsceZIGipZCj/CKYvYEBkGNWM6ZhvhrYk5sezwQ2FDjG9UT 98UMjhNi26R9Jl0yBwnyJIPzJXyBA9fnryfOErFE= X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.99.2 at mail.thecshore.com From: cshored@thecshore.com To: openwrt-devel@lists.openwrt.org Date: Fri, 2 Feb 2018 02:29:15 -0500 Message-Id: <20180202072915.7257-1-cshored@thecshore.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20180130193335.22225-1-cshored@thecshore.com> References: <20180130193335.22225-1-cshored@thecshore.com> Subject: [OpenWrt-Devel] [PATCH v3] [netifd] vlan: Buffer overlow in snprintf for vlans X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openwrt-devel-bounces@lists.openwrt.org Sender: "openwrt-devel" From: "Daniel F. Dickinson" Buffer overflow condition can occur because vlan device name is constructed from device name (size IFNAMSIZ) plus the ASCII decimal representation of the vlan id plus a dot, but the target can only be IFNAMSIZ. Note that the generic device name code must also be updated or vlan id's may be truncated for device like vlan on top of bridges. Credit to "Andrey Melnikov" for a better solution than the original patch I sent for the vlan.c part. Signed-off-by: Daniel F. Dickinson --- device.c | 11 ++++++++++- vlan.c | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/device.c b/device.c index a851037..9f8ffa2 100644 --- a/device.c +++ b/device.c @@ -660,6 +660,7 @@ void device_set_ifindex(struct device *dev, int ifindex) int device_set_ifname(struct device *dev, const char *name) { int ret = 0; + char *vlandot = 0; if (!strcmp(dev->ifname, name)) return 0; @@ -667,7 +668,15 @@ int device_set_ifname(struct device *dev, const char *name) if (dev->avl.key) avl_delete(&devices, &dev->avl); - strncpy(dev->ifname, name, IFNAMSIZ); + if ((vlandot = strchr(name, '.'))) { + /* Copy the part of name that will leave space for a vlan id + * the dot, and terminating null. NB. max vlan id is 4095 + */ + strncpy(dev->ifname, name, IFNAMSIZ - strnlen(vlandot, 5) - 1); + strncat(dev->ifname, vlandot, 5); + } else { + strncpy(dev->ifname, name, IFNAMSIZ); + } if (dev->avl.key) ret = avl_insert(&devices, &dev->avl); diff --git a/vlan.c b/vlan.c index 067f624..8bacc8f 100644 --- a/vlan.c +++ b/vlan.c @@ -66,7 +66,8 @@ static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev) char name[IFNAMSIZ]; vldev->dev.hidden = dev->hidden; - snprintf(name, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id); + /* Truncate dev->ifname so that adding dot + VLAN id + '\0' doesn't exceed IFNAMSIZ */ + snprintf(name, sizeof(name), "%.*s.%d", IFNAMSIZ - 6, dev->ifname, vldev->id & 0xfff); device_set_ifname(&vldev->dev, name); }