From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id sJ1kJL05Xmm1eDAAWB0awg (envelope-from ) for ; Wed, 07 Jan 2026 05:47:25 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=XzE4Fj1c; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 877F61E0B6; Wed, 07 Jan 2026 05:47:25 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id DFBBE1E048 for ; Wed, 07 Jan 2026 05:47:24 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 59D354BA2E21 for ; Wed, 7 Jan 2026 10:47:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 59D354BA2E21 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=XzE4Fj1c Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 5C78E4BA2E2D for ; Wed, 7 Jan 2026 10:46:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5C78E4BA2E2D Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5C78E4BA2E2D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767782818; cv=none; b=RN+MpPGtLlRpo59LdQCNO6DXnIR4sotEH06OkoCyCZhsWeRLF1Jdnwe2nsyB+SWf8z5Dd6pnzL6imDcYCEeM2/rI/zNRY/sGfvsK5/RivVCfr2EfNpiIKHArnWFBLy++e4fn2fU1g11lEZylFsMjQ9U34AoBh3zZeQJwW3imUeg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767782818; c=relaxed/simple; bh=XPDvnh+ry+ffRccoZBXqQm72eL0VA+ThfZjVkXEsTjU=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=lgeoMpHWLdQw7ctkqCvhSaM6R8d7s2Ua2ETq70pyazcHyfgZWjSpUrHXbWSoB53EvFtHAZQNiSMUWWJJt6Izh1qI+Axj+JdWpVp6KGnnTIZQERVlCfhzaazbVZlJG0d1EeVQySLl7GA9+TZw7Q4JDNvnitHJ52rYrumgnHYu4GE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5C78E4BA2E2D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1767782818; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zIIHYMFBo17N+BXI8oIWT2IKzB/l3M4md7cv+Edpibk=; b=XzE4Fj1cQ4HHZD+WhFU1F/ZllZ8o9bvfwBuJ6TJ2b4cjAwaU20qYs60vr0lDyutZX6+1Dc QxpLeFpJZYPNNTW0QvG5MEPBxaOSfFY9lKaX4Fu6qSnI2EfQQWqGQJe8FNNhXrpDfTgHwh kxLdqUlxKuNCCaDuUyWGISrEO4HBGNA= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-676-Ef09-23MPLamTEtiSSVyfA-1; Wed, 07 Jan 2026 05:46:57 -0500 X-MC-Unique: Ef09-23MPLamTEtiSSVyfA-1 X-Mimecast-MFC-AGG-ID: Ef09-23MPLamTEtiSSVyfA_1767782816 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-47a97b719ccso11898205e9.2 for ; Wed, 07 Jan 2026 02:46:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767782815; x=1768387615; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zIIHYMFBo17N+BXI8oIWT2IKzB/l3M4md7cv+Edpibk=; b=kEXVbttKxwkakFnqjYn47B8h/ypeHAyZwZgG8fDaXbLJthNVjJhvwMwJjnDRAko5Yx G2CiWbVirjkq4Na/tiuu4SsSnPGV7rNYYO1pBPzSQ/LJ74rTndC5pbVXpjB397/w2+un 9x9oRXouwTdJFL0wrpbj28kdA45tIRqKd+LrNLuWJZ8Q36gHGXJp4B/cna3Qazc6ZHUp wlKTgsr/IYWM9O0KNx5Ww3Gkfi/P3c6esZTZ4QPYQJQH2Zspy/S3MqCUwS2hGOFezaU/ VEtIWNxZ/5CXYpw3fQBGzEtJwTa0y3KHRJDVI7Rg1FUVpRVrm07Q88TZc4BoecyBMdGJ 9o+g== X-Forwarded-Encrypted: i=1; AJvYcCWCVGQOt0bTis0zuVgSCvI89joSuar5Y6JfEC61W2fomBFtznJfmF25f4mV4jZRMuPm0EJZHT8fJn1+VA==@sourceware.org X-Gm-Message-State: AOJu0YxPK0uuS3V5w3zTNSlHwzuC+nwy1QsmvKQ93WrYxiLG+Xw948Yl 61gV7IOcGTgestOHHtKGll9byShAU395E7RefMu49ugNqW4GV2hoYL4LrUfryQTmFO2icI1kGI/ NjMKizVGcVgPKMJuN4UM1AfM8qQ1WlBovnNz9GheK2CtmBjwbxmQxzF5/mUsP7PpdL1Bmojk= X-Gm-Gg: AY/fxX6INYfLfYNK4ncrILy+W+gkDaXaryAZX/tIwnpLPt7AuXrs1T0tDKsb4Nyru/h 3rj9eccJF8lXp7C0TxnjTRzgGhNpssKl9VEziAQtvIHkFNXqExtl8ljvbhk5qX5McrC3vT5QrQf Br/cMyzctqDH5Yl1ED2/Wd67fNHHKCSwPDtT+SwMAp+C5h2xH3RFhAteA85vk8tLEZFzFiYhbZi KVvwZf3q+psck2nhnRfHlgnF4XepBiy34M2hPhDTn1WvgW9sMeH5zX7ZH/62epdZU7Xs8yelAkI DKogkaqlzsVvIqRDmZJvriiC//7VqdmoBwMNmXSKvCalNYLvxn1Gy4KGOqOma2VH36XrHpd/DDU ET8+/rnDgfw3ZnLpX3D2EKD5/YhiM X-Received: by 2002:a05:600c:450f:b0:479:3a89:121e with SMTP id 5b1f17b1804b1-47d84b614c6mr22649375e9.37.1767782815446; Wed, 07 Jan 2026 02:46:55 -0800 (PST) X-Google-Smtp-Source: AGHT+IFIY/vRDWwjzUqW9WPTe7AYG95t0486YhODtzPkAIrXISwIbUYO7DQSHl24RihjD2DqfABlKg== X-Received: by 2002:a05:600c:450f:b0:479:3a89:121e with SMTP id 5b1f17b1804b1-47d84b614c6mr22649035e9.37.1767782814987; Wed, 07 Jan 2026 02:46:54 -0800 (PST) Received: from localhost (84.81.93.209.dyn.plus.net. [209.93.81.84]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47d7f390a69sm94045895e9.0.2026.01.07.02.46.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Jan 2026 02:46:54 -0800 (PST) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH] [gdb] Fix heap-buffer-overflow in args_complete_p In-Reply-To: <7beac4be-7924-48b5-804b-6400efd02834@suse.de> References: <20260103145559.2722584-1-tdevries@suse.de> <874iozygr7.fsf@redhat.com> <7beac4be-7924-48b5-804b-6400efd02834@suse.de> Date: Wed, 07 Jan 2026 10:46:53 +0000 Message-ID: <875x9dwvhe.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: csDNeHxjbDudEpG05Y-3hj6HHLlnHKl5bLJAjLXPtEo_1767782816 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom de Vries writes: > On 1/5/26 8:57 PM, Andrew Burgess wrote: >> Tom de Vries writes: >> >>> PR gdb/33754 reports a heap-buffer-overflow here in args_complete_p: >>> ... >>> while (*input != '\0') >>> ... >>> >>> Fix this by introducing a lambda function at that safely handles all char >>> array accesses. >> >> Sorry to be a bore, but after reading this commit, and the bug report, >> it's still not obvious to me where the overflow actually occurs. >> >> I totally accept that this code is broken, but as I introduced this bug, >> I wanted to learn from this mistake, but this commit doesn't really >> explain what mistake is being fixed. >> >> Do you think you could explain what's actually going wrong here? >> > > Hi Andrew, > > agreed, it's not spelled out, sorry about that. > > So, the heap-buffer-overflow happens with: > ... > (gdb) p args > $1 = "\"first arg\" \"\" \"third-arg\" \"'\" \"\\\"\" \" \" \"\" " > ... > and it's the fact that we don't check for '\0' after skip_spaces that is > the problem. I think it should be possible to reproduce the problem > with args == " ". Thanks for breaking it down for me. I don't really like the original lambda function approach that was proposed, I'd prefer to just see the correct checks added to the loop. More inline below... > > So a minimal fix for this problem is: > ... > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 1a7daf1461b..fdcd4e4ba96 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -131,6 +131,8 @@ args_complete_p (const std::string &args) > while (*input != '\0') > { > input = skip_spaces (input); > + if (*input == '\0') > + break; I think I prefer this to Tom's proposed 'for' loop, but I don't feel super strongly each way. > > if (squote) > { > ... > > But the strchr problem is also there, so this: > ... > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 1a7daf1461b..4bcd523f79b 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -177,6 +177,8 @@ args_complete_p (const std::string &args) > dquote = true; > } > > + if (*input == '\0') > + break; I'd replace this with 'gdb_assert (*input != '\0');', and then use something like the extra check I proposed next to the strchr calls. Or maybe we should add a new helper function in gdbsupport/ like: static char * strchr_not_null (char *s, int c) { if (c == '\0') return nullptr; return strchr (s, c); } static const char * strchr_not_null (const char *s, int c) { return strchr_not_null (const_cast (s), c); } which wraps the null check. Either would be fine with me. I also liked the selftests you added, I extended them to: static void check_str (const std::string &str, bool complete_p) { const char *end; SELF_CHECK (args_complete_p (str, &end) == complete_p); SELF_CHECK (end == str.data () + str.size ()); } static void infcmd_args_complete_p_tests (void) { check_str (" ", true); check_str ("\\", true); check_str ("\"\\", false); } which covers all the bugs that are being fixed here. Thanks, Andrew