From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751533AbdHSFgi (ORCPT ); Sat, 19 Aug 2017 01:36:38 -0400 Received: from mx2.suse.de ([195.135.220.15]:33259 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751107AbdHSFgh (ORCPT ); Sat, 19 Aug 2017 01:36:37 -0400 Date: Sat, 19 Aug 2017 07:36:34 +0200 Message-ID: From: Takashi Iwai To: "Lee, Chun-Yi" Cc: David Howells , linux-kernel@vger.kernel.org, "Lee, Chun-Yi" , Rusty Russell , Pawel Wieczorkiewicz Subject: Re: [PATCH] X.509: Fix the buffer overflow in the utility function for OID string In-Reply-To: <20170819041944.11855-1-jlee@suse.com> References: <20170819041944.11855-1-jlee@suse.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 19 Aug 2017 06:19:44 +0200, Lee, Chun-Yi wrote: > > From: Takashi Iwai > > The sprint_oid() utility function doesn't properly check the buffer > size that it causes that the warning in vsnprintf() be triggered. > For example on v4.1 kernel: > > [ 49.612536] ------------[ cut here ]------------ > [ 49.612543] WARNING: CPU: 0 PID: 2357 at lib/vsprintf.c:1867 vsnprintf+0x5a7/0x5c0() > ... > > We can trigger this issue by injecting maliciously crafted x509 cert > in DER format. Just using hex editor to change the length of OID to > over the length of the SEQUENCE container. For example: > > 0:d=0 hl=4 l= 980 cons: SEQUENCE > 4:d=1 hl=4 l= 700 cons: SEQUENCE > 8:d=2 hl=2 l= 3 cons: cont [ 0 ] > 10:d=3 hl=2 l= 1 prim: INTEGER :02 > 13:d=2 hl=2 l= 9 prim: INTEGER :9B47FAF791E7D1E3 > 24:d=2 hl=2 l= 13 cons: SEQUENCE > 26:d=3 hl=2 l= 9 prim: OBJECT :sha256WithRSAEncryption > 37:d=3 hl=2 l= 0 prim: NULL > 39:d=2 hl=2 l= 121 cons: SEQUENCE > 41:d=3 hl=2 l= 22 cons: SET > 43:d=4 hl=2 l= 20 cons: SEQUENCE <=== the SEQ length is 20 > 45:d=5 hl=2 l= 3 prim: OBJECT :organizationName > <=== the original length is 3, change the length of OID to over the length of SEQUENCE > > Pawel Wieczorkiewicz reported this problem and Takashi Iwai provided > patch to fix it by checking the bufsize in sprint_oid(). > > From: Takashi Iwai > Reported-by: Pawel Wieczorkiewicz > Cc: David Howells > Cc: Rusty Russell > Cc: Takashi Iwai > Cc: Pawel Wieczorkiewicz > Signed-off-by: "Lee, Chun-Yi" I seem to have forgotten to give my sign-off: Signed-off-by: Takashi Iwai thanks, Takashi > --- > lib/oid_registry.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/oid_registry.c b/lib/oid_registry.c > index 318f382..41b9e50 100644 > --- a/lib/oid_registry.c > +++ b/lib/oid_registry.c > @@ -142,9 +142,9 @@ int sprint_oid(const void *data, size_t datasize, char *buffer, size_t bufsize) > } > ret += count = snprintf(buffer, bufsize, ".%lu", num); > buffer += count; > - bufsize -= count; > - if (bufsize == 0) > + if (bufsize <= count) > return -ENOBUFS; > + bufsize -= count; > } > > return ret; > -- > 2.10.2 >