From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id C62323858D3C for ; Sat, 24 Sep 2022 08:58:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C62323858D3C Received: by mail-wr1-x433.google.com with SMTP id g3so3154290wrq.13 for ; Sat, 24 Sep 2022 01:58:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date; bh=iyO60DgEY4d/vZxcrbORy0FhOmxuEptELSFoy44NGYY=; b=XqhkJ5YY5Yzj2eUGMrzanx9ReYXvdCYZg1fP55U6fI6a5xX++xVK0nKOfDmgIvTn/P UoE3I/+Mp25IDbTuexPymAbWgNwNor/vaEahC56jyLgZSDEjGhZrJ7UIfT2QUsCYIKDD 3IqkPStn+zRru5FKXDTR2XuZa4aIqn9wE8LG+S4FvKIyHUVBB1iPlLbVyha7YnhYjp8y 1PeM0rC4t8YTD6RvdvsDFibis3L0NVWioQOMgzVAAEL4Y6R2mF1kPno4Eq8MPi09HqjO 5YoVWYmd9fr5ztxHP5c72mR7YfroEwiHofyHSfEZxn7Mof69MAUwekZMJO8BF2YIzbF3 5BRQ== X-Gm-Message-State: ACrzQf0Ud5Vcz15f5ij64lEA9egHvhO34TucNxn3ollhKrg95ioz180F rD/BWdIrxscfs/DfPadH77rHugs7l+3rI7E7bmA5OzQ2rBNx959hTYgZPl5YLw2X+jZLpaL1B2v lfWmppx7+BSn9NQF9NTeMwAF4CZcn8d9BxI27xQxqrlkufn02f2imGJepHSPN+iryGw== X-Google-Smtp-Source: AMsMyM7MY8WWG6P8zTyPEgGUkwzZ7ZpnfCjxH3xpkyrYZ5FLZbOFU9I4Md7Q5bjeuSR3oro+qA41mg== X-Received: by 2002:a5d:4301:0:b0:21b:8af6:4a21 with SMTP id h1-20020a5d4301000000b0021b8af64a21mr7927371wrq.296.1664009898377; Sat, 24 Sep 2022 01:58:18 -0700 (PDT) Received: from sbrinz-thinkpad (nrwh-14-b2-v4wan-164652-cust345.vm23.cable.virginm.net. [81.96.125.90]) by smtp.gmail.com with ESMTPSA id n11-20020adfe34b000000b002252ec781f7sm9246079wrj.8.2022.09.24.01.58.17 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 24 Sep 2022 01:58:17 -0700 (PDT) From: Magne Hov To: gdb-patches@sourceware.org Subject: Re: [PATCH v2] gdb/source.c: Fix undefined behaviour dereferencing empty string In-Reply-To: <20220921145834.837969-1-mhov@undo.io> References: <20220915183141.3484234-1-mhov@undo.io> <20220921145834.837969-1-mhov@undo.io> Date: Sat, 24 Sep 2022 09:58:16 +0100 Message-ID: <5szgepjgif.fsf@undo.io> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 24 Sep 2022 08:58:22 -0000 On Wed, Sep 21 2022, Magne Hov wrote: > When a source file's dirname is solely made up of directory separators > we end up trying to dereference the last character of an empty string > with std::string::back, which results in undefined behaviour. A typical > use case where this can happen is when the root directory "/" is used as > a compilation directory. > > With libstdc++.so.6.0.28 we get no out-of-bounds checks and the byte > preceding the storage of the empty string is returned. The character > value of this byte depends on heap implementation and usage, but when > this byte happens to hold the value of the directory separator character > we go on to call std::string::pop_back on the empty string which results > in an out_of_range exception which terminates GDB. > > Fix this by using path_join. prepare_path_for_appending ensures that the > filename component is relative. > > The testsuite has been run before and after the change and no > regressions were found. > --- > gdb/source.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/gdb/source.c b/gdb/source.c > index 3f498d552c4..25ad1ecb3da 100644 > --- a/gdb/source.c > +++ b/gdb/source.c > @@ -1146,15 +1146,7 @@ find_and_open_source (const char *filename, > helpful if part of the compilation directory was removed, > e.g. using gcc's -fdebug-prefix-map, and we have added the missing > prefix to source_path. */ > - std::string cdir_filename (dirname); > - > - /* Remove any trailing directory separators. */ > - while (IS_DIR_SEPARATOR (cdir_filename.back ())) > - cdir_filename.pop_back (); > - > - /* Add our own directory separator. */ > - cdir_filename.append (SLASH_STRING); > - cdir_filename.append (filename_start); > + std::string cdir_filename = path_join (dirname, filename_start); > > result = openp (path, OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH, > cdir_filename.c_str (), OPEN_MODE, fullname); > -- > 2.25.1 > This has been pushed now. Thank you all for the review. -- Magne Hov | Software Engineer | Direct: +44 7395 395 648 | mhov@undo.io Undo | Record. Replay. Resolve