From patchwork Thu Aug 18 02:06:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Neels Hofmeyr X-Patchwork-Id: 660308 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.osmocom.org (lists.osmocom.org [144.76.43.76]) by ozlabs.org (Postfix) with ESMTP id 3sF8bf4NDxz9t17 for ; Thu, 18 Aug 2016 12:06:38 +1000 (AEST) Received: from lists.osmocom.org (lists.osmocom.org [144.76.43.76]) by lists.osmocom.org (Postfix) with ESMTP id D5686264D4; Thu, 18 Aug 2016 02:06:35 +0000 (UTC) X-Original-To: openbsc@lists.osmocom.org Delivered-To: openbsc@lists.osmocom.org Received: from einhorn.in-berlin.de (einhorn.in-berlin.de [IPv6:2001:bf0:c000::1:8]) by lists.osmocom.org (Postfix) with ESMTP id 16DD2264B6 for ; Thu, 18 Aug 2016 02:06:33 +0000 (UTC) X-Envelope-From: nhofmeyr@sysmocom.de X-Envelope-To: Received: from localhost (cable-37-120-33-11.cust.telecolumbus.net [37.120.33.11] (may be forged)) (authenticated bits=0) by einhorn.in-berlin.de (8.14.4/8.14.4/Debian-8) with ESMTP id u7I26WiN009607 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 18 Aug 2016 04:06:32 +0200 From: Neels Hofmeyr To: openbsc@lists.osmocom.org Subject: [openggsn] [PATCH 1/2] Fix possible buffer overflow for gsn_restart file path Date: Thu, 18 Aug 2016 04:06:14 +0200 Message-Id: <1471485975-1775-1-git-send-email-nhofmeyr@sysmocom.de> X-Mailer: git-send-email 2.1.4 X-BeenThere: openbsc@lists.osmocom.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "Development of OpenBSC, OsmoBSC, OsmoNITB, OsmoCSCN" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openbsc-bounces@lists.osmocom.org Sender: "OpenBSC" For strncat, to obtain n, one must not subtract the length of what is appended, but the length of what is already written from the buffer size. Verified with this little test program: #include #include int main() { char buf[20]; strncpy(buf, "123", 10); strncat(buf, "456789012345", 10 - strlen(buf)); printf("%s\n", buf); return 0; } It prints "1234567890". --- gtp/gtp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gtp/gtp.c b/gtp/gtp.c index 12cb492..34e1dc6 100644 --- a/gtp/gtp.c +++ b/gtp/gtp.c @@ -648,9 +648,10 @@ static void log_restart(struct gsn_t *gsn) int counter = 0; char filename[NAMESIZE]; - filename[NAMESIZE - 1] = 0; /* No null term. guarantee by strncpy */ + /* guarantee nul term, strncpy might omit if too long */ + filename[NAMESIZE - 1] = 0; strncpy(filename, gsn->statedir, NAMESIZE - 1); - strncat(filename, RESTART_FILE, NAMESIZE - 1 - sizeof(RESTART_FILE)); + strncat(filename, RESTART_FILE, NAMESIZE - 1 - strlen(filename)); i = umask(022);