ALT Linux Team development discussions
 help / color / mirror / Atom feed
From: Ivan Zakharyaschev <imz@altlinux.org>
To: devel@lists.altlinux.org
Cc: darktemplar@altlinux.org, ldv@altlinux.org
Subject: [devel] [APT PATCH] rpmSingle{Pkg, Src}Index::ArchiveURI(): avoid cases with undefined behavior
Date: Sun, 16 Feb 2020 04:09:14 +0300 (MSK)
Message-ID: <alpine.LFD.2.20.2002160401421.6363@imap.altlinux.org> (raw)

Two cases of UB are avoided with such a rewrite:

* getcwd(2) returned NULL. Constructing a string from NULL is UB.
  (Such string was passed as an argument to flCombine().)
  Now, SafeGetCwd() (in fileutl.cc) returns "/" in such cases;
  if you consider SafeGetCwd()'s implementation not to be reasonable,
  rewrite it (just at a single place).

* File.length() < 2. Since File was a non-const string,
  File[File.length()] might be UB before C++11. Now, File is const, and
  it is guaranteed that File[File.length()] == 0.

This change was inspired by:

commit c138c48501381d26872a6b18aa1fb7af9f0300b2
Author: Aleksei Nikiforov <darktemplar@altlinux.org>
Date:   Mon Jun 10 18:10:30 2019 +0300

    RPM ArchiveURI: check file length before using it
---
 apt/apt-pkg/rpm/rpmindexfile.cc | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/apt/apt-pkg/rpm/rpmindexfile.cc b/apt/apt-pkg/rpm/rpmindexfile.cc
index 901ba77dd..ad6899a08 100644
--- a/apt/apt-pkg/rpm/rpmindexfile.cc
+++ b/apt/apt-pkg/rpm/rpmindexfile.cc
@@ -511,26 +511,20 @@ unsigned long rpmSrcDirIndex::Size() const
 
 // SinglePkgIndex::ArchiveURI - URI for the archive       	        /*{{{*/
 // ---------------------------------------------------------------------
-string rpmSinglePkgIndex::ArchiveURI(string File) const
+string rpmSinglePkgIndex::ArchiveURI(const string File) const
 {
-   char *cwd = getcwd(NULL,0);
-   if (File[0] == '.' && File[1] == '/')
-      File = string(File, 2);
-   string URI = "file://"+flCombine(cwd, File);
-   free(cwd);
-   return URI;
+   return "file://"+
+      flCombine(SafeGetCWD(),
+                (File[0] == '.' && File[1] == '/') ? string(File, 2) : File);
 }
 									/*}}}*/
 // SinglePkgIndex::ArchiveURI - URI for the archive       	        /*{{{*/
 // ---------------------------------------------------------------------
-string rpmSingleSrcIndex::ArchiveURI(string File) const
-{
-   char *cwd = getcwd(NULL,0);
-   if (File[0] == '.' && File[1] == '/')
-      File = string(File, 2);
-   string URI = "file://"+flCombine(cwd, File);
-   free(cwd);
-   return URI;
+string rpmSingleSrcIndex::ArchiveURI(const string File) const
+{
+   return "file://"+
+      flCombine(SafeGetCWD(),
+                (File[0] == '.' && File[1] == '/') ? string(File, 2) : File);
 }
 									/*}}}*/
 
-- 
2.21.0


             reply	other threads:[~2020-02-16  1:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-16  1:09 Ivan Zakharyaschev [this message]
2020-02-16 11:17 ` Dmitry V. Levin
2020-02-16 17:25   ` [devel] [nitpick] " Michael Shigorin
2020-02-17  0:39   ` [devel] " Ivan Zakharyaschev
2020-02-18  1:55     ` Ivan Zakharyaschev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.20.2002160401421.6363@imap.altlinux.org \
    --to=imz@altlinux.org \
    --cc=darktemplar@altlinux.org \
    --cc=devel@lists.altlinux.org \
    --cc=ldv@altlinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

ALT Linux Team development discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
		devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
	public-inbox-index devel

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git