From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id nbMoHAV2XmkO3TAAWB0awg (envelope-from ) for ; Wed, 07 Jan 2026 10:04:37 -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=J+1Q238k; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 611A01E0B6; Wed, 07 Jan 2026 10:04:37 -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 A3B921E048 for ; Wed, 07 Jan 2026 10:04:36 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 0D6334BA2E29 for ; Wed, 7 Jan 2026 15:04:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0D6334BA2E29 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=J+1Q238k 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 B69DD4BA23DC for ; Wed, 7 Jan 2026 15:01:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B69DD4BA23DC 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 B69DD4BA23DC 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=1767798077; cv=none; b=EU7hgaUDAxAhYdUGm38dvkTfjGRypBFolRltE0cj9F4vYHpnNxnxEv64RCN6D/f76DVI/WLSVhzYLa5Hxn4s7M7lJW5tAGL9rX5cBmJqDeGrtcYk96SlAq/cSTJQFto8ZhAa6iS3S90u85loWshW4M1jZbhus/Pc9DTy3Tu1KqM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767798077; c=relaxed/simple; bh=7S4ZFgIatkRt1VPhEEscr8jP//hxLQwh4tPkjICXYpI=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=L3N3ULB62mDwuaJz3VpK3uUaNP9Zzs0ru+6ayAxXUdG8Mbmxqgu1wot0z1eVRILft+nqMK+OmzaX6q2dWdWoTAj0rrLEQxMxRctP9GlsN3yi8wWwhgZPdgTzD+nU/W3FGxhHmVtrzOr450XJOHuL9cEh49kXnv/5JaFWtVi8HwY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B69DD4BA23DC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1767798077; 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=29wS/6vS+wl1QkV3O6BHNncseAk2OYBkSB/4ES80q8k=; b=J+1Q238knjIc3GsY72izVY+Hfm4ljHiAjWjJ20PKbIOHXAHKLPIKA/GVJYf1tV0pIB//rQ mureTuhmadEM32kljm+M8l8OHlGpRGxxDlJyLhvavMnZKNaah+L2yuX5A6oQTB6ShgJVDT Qqbo3NqRBsFouFSZX8zkiXVDS5rliCM= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-676-bPfEYprPO1C5jEi3epJYDA-1; Wed, 07 Jan 2026 10:01:16 -0500 X-MC-Unique: bPfEYprPO1C5jEi3epJYDA-1 X-Mimecast-MFC-AGG-ID: bPfEYprPO1C5jEi3epJYDA_1767798075 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-477b8a667bcso25256215e9.2 for ; Wed, 07 Jan 2026 07:01:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767798075; x=1768402875; 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=29wS/6vS+wl1QkV3O6BHNncseAk2OYBkSB/4ES80q8k=; b=wmDG1j0Dcp4Yjp4QktrWi8Fri9XoKObMdnKhXNiDConrxsSK6jVdBt7RJKtrePmFsp LYj3k5mK6TIxDgT69H5lCcA7QWN/uEtElHN1tA4qY4gZ4LJVC4GRpA1rRtrmT8C73lKy gRmlJcKRKW/Ckfy1iOQC2YjG5m4Aa/C+6+xpqxTpspH8AV94F+QYTk1V8ZoVsHr3/O+0 JMKFwnZvH3se30DoC4v1mmlSaZ+pFgtO3G8r130AjbbnTxAJR/fxUG73LklnP+6pI92M 6TYoPxA3pr9GCcp0gEDTrUYXeiF1JidL2tG1VwOpkspYvqDq12cGnsQ5KrokL9IPiwOm nD3Q== X-Forwarded-Encrypted: i=1; AJvYcCUIV1sI7bMxZF2xlpd+Wsmturx28wRw5NZ3466zQa6lAiF0L1wwohmSBQmcqZoTNSUgmk5glNnltxj9Jg==@sourceware.org X-Gm-Message-State: AOJu0Yxd5LE8O7rIvAKT12/6gpmglGtvtgKQx+sddIKSqe4ChjcLL6/E 8mktx0FNq25UnuABx1Te8j1a3+yaZuKwzgc6m2j5s4WWW0k8j1wlzfZY2RoG4qlFYBf/2wwVByN 2ZxkSMC2uDIJsR5DxpylswGDLmXo85Q/kF7CJBCCt+bakzl9fWyaaulAlpSqIUVw= X-Gm-Gg: AY/fxX6rdVC8ND6CJWwAixdAfh+I4HX3GFOyb3FgD2n2BpEmaiQeI9whTTI1ZH+bF6S 7h9kfzqB2h7wEyaeOtrGON3SnNGLbkiw3LG7Vj/2yma3vzkSCTrz6IAQNVYPo/UTyDYYv0NmpQG gZr9Jrll7C/MDCRjcmZX+tQ1aazdRqjJWg7ItV/cNoEwQSf5UZgIcmr/MhDENG9oO6+El6ozUfl DDcg+kaQ1cL+S8QvBNa4EMNgbpQwMLEuuw97SIqOXJKw4QhU2PPue0uDToXBUM9ttNheeijafpK sIHnyK7KEqRer4YDiUeytDenW7c4/ckISWNM8cvpCIIank8dLi6qZIsv/Gn8ST8ehF/zh9/RaVB XKASPQQyV9KUeJVA/Vy3/xsX5B7zn X-Received: by 2002:a05:600d:103:b0:47d:403e:9cd5 with SMTP id 5b1f17b1804b1-47d84b1fce2mr26261285e9.11.1767798073859; Wed, 07 Jan 2026 07:01:13 -0800 (PST) X-Google-Smtp-Source: AGHT+IEJyejqqyR2WCjKrvYU7Kavell/wlh2bwJUu1JiClm3tnNPCpikHVHVIlSbaX+1vFkkK0eFFg== X-Received: by 2002:a05:600d:103:b0:47d:403e:9cd5 with SMTP id 5b1f17b1804b1-47d84b1fce2mr26260185e9.11.1767798073012; Wed, 07 Jan 2026 07:01:13 -0800 (PST) Received: from localhost (84.81.93.209.dyn.plus.net. [209.93.81.84]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47d871ac28bsm13596475e9.20.2026.01.07.07.01.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Jan 2026 07:01:12 -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: References: <20260103145559.2722584-1-tdevries@suse.de> <874iozygr7.fsf@redhat.com> <7beac4be-7924-48b5-804b-6400efd02834@suse.de> <875x9dwvhe.fsf@redhat.com> Date: Wed, 07 Jan 2026 15:01:10 +0000 Message-ID: <87tswxv555.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: kkjrO7LOJDmw9DW9Q8cUvianjeF79-HyqVeWmgwnZCI_1767798075 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/7/26 11:46 AM, Andrew Burgess wrote: >> 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. >> > > Hi Andrew, > > thanks for the comments. > > But by now, a v2 was submitted, approved and committed. > > So perhaps you want to submit a refactoring patch addressing some of > your insights here. Otherwise, I can take it further. Let me know what > you prefer. No, that's fine. I do with more people would post vN patches as follow ups to their original messages, it would make tracking revisions so much easier. Maybe one day we'll move off the mailing list model. Thanks, Andrew