diff options
author | David Kalnischkies <david@kalnischkies.de> | 2020-02-20 12:49:15 +0100 |
---|---|---|
committer | Julian Andres Klode <julian.klode@canonical.com> | 2020-02-26 18:12:18 +0100 |
commit | 110078022a6c6103be8f557aef1e268c4b680d88 (patch) | |
tree | b2a29c48a2509816247fdc52e9dc6f0502b14c79 | |
parent | 6bb1c85fd045e421be36957453ee318c0cea8fb0 (diff) |
Parse records including empty tag names correctly
No sensible file should include these, but even insensible files do not
gain unfair advantages with it as this parser does not deal with
security critical files before they haven't passed other checks like
signatures or hashsums.
The problem is that the parser accepts and parses empty tag names
correctly, but does not store the data parsed which will effect later
passes over the data resulting e.g. in the following tag containing
the name and value of the previous (empty) tag, its own tagname and its
own value or a crash due to an attempt to access invalid memory
depending on who passes over the data and what is done with it.
This commit fixes both, the incidient of the crash reported by
Anatoly Trosinenko who reproduced it via apt-sortpkgs:
| $ cat /tmp/Packages-null
| 0:
| PACKAGE:0
|
| :
| PACKAGE:
|
| PACKAGE::
| $ apt-sortpkgs /tmp/Packages-null
and the deeper parsing issue shown by the included testcase.
Reported-By: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
References: 8710a36a01c0cb1648926792c2ad05185535558e
-rw-r--r-- | apt-pkg/tagfile.cc | 5 | ||||
-rw-r--r-- | test/libapt/tagfile_test.cc | 58 |
2 files changed, 60 insertions, 3 deletions
diff --git a/apt-pkg/tagfile.cc b/apt-pkg/tagfile.cc index 0f0d8c9a7..52000c6b9 100644 --- a/apt-pkg/tagfile.cc +++ b/apt-pkg/tagfile.cc @@ -513,7 +513,6 @@ bool pkgTagSection::Scan(const char *Start,unsigned long MaxLength, bool const R return false; pkgTagSectionPrivate::TagData lastTagData(0); - lastTagData.EndTag = 0; Key lastTagKey = Key::Unknown; unsigned int lastTagHash = 0; while (Stop < End) @@ -529,7 +528,7 @@ bool pkgTagSection::Scan(const char *Start,unsigned long MaxLength, bool const R if (isspace_ascii(Stop[0]) == 0) { // store the last found tag - if (lastTagData.EndTag != 0) + if (lastTagData.StartValue != 0) { if (lastTagKey != Key::Unknown) { AlphaIndexes[static_cast<size_t>(lastTagKey)] = TagCount; @@ -579,7 +578,7 @@ bool pkgTagSection::Scan(const char *Start,unsigned long MaxLength, bool const R // Double newline marks the end of the record if (Stop+1 < End && Stop[1] == '\n') { - if (lastTagData.EndTag != 0) + if (lastTagData.StartValue != 0) { if (lastTagKey != Key::Unknown) { AlphaIndexes[static_cast<size_t>(lastTagKey)] = TagCount; diff --git a/test/libapt/tagfile_test.cc b/test/libapt/tagfile_test.cc index 8823ff781..919b46cdb 100644 --- a/test/libapt/tagfile_test.cc +++ b/test/libapt/tagfile_test.cc @@ -285,3 +285,61 @@ TEST(TagFileTest, Comments) EXPECT_FALSE(tfile.Step(section)); } + +TEST(TagFileTest, EmptyTagName) +{ + FileFd fd; + createTemporaryFile("emptytagname", fd, NULL, "0:\n" +"PACKAGE:0\n" +"\n" +":\n" +"PACKAGE:\n" +"\n" +"PACKAGE:\n" +":\n" +"\n" +"PACKAGE:\n" +":\n" +"Version:1\n" +"\n" +"PACKAGE::\n" + ); + pkgTagFile tfile(&fd); + pkgTagSection section; + ASSERT_TRUE(tfile.Step(section)); + EXPECT_EQ(2u, section.Count()); + EXPECT_TRUE(section.Exists("PACKAGE")); + EXPECT_EQ("0", section.FindS("PACKAGE")); + EXPECT_TRUE(section.Exists("0")); + EXPECT_EQ("", section.FindS("0")); + + ASSERT_TRUE(tfile.Step(section)); + EXPECT_EQ(2u, section.Count()); + EXPECT_TRUE(section.Exists("PACKAGE")); + EXPECT_EQ("", section.FindS("PACKAGE")); + EXPECT_TRUE(section.Exists("")); + EXPECT_EQ("", section.FindS("")); + + ASSERT_TRUE(tfile.Step(section)); + EXPECT_EQ(2u, section.Count()); + EXPECT_TRUE(section.Exists("PACKAGE")); + EXPECT_EQ("", section.FindS("PACKAGE")); + EXPECT_TRUE(section.Exists("")); + EXPECT_EQ("", section.FindS("")); + + ASSERT_TRUE(tfile.Step(section)); + EXPECT_EQ(3u, section.Count()); + EXPECT_TRUE(section.Exists("PACKAGE")); + EXPECT_EQ("", section.FindS("PACKAGE")); + EXPECT_TRUE(section.Exists("")); + EXPECT_EQ("", section.FindS("")); + EXPECT_TRUE(section.Exists("Version")); + EXPECT_EQ("1", section.FindS("Version")); + + ASSERT_TRUE(tfile.Step(section)); + EXPECT_EQ(1u, section.Count()); + EXPECT_TRUE(section.Exists("PACKAGE")); + EXPECT_EQ(":", section.FindS("PACKAGE")); + + EXPECT_FALSE(tfile.Step(section)); +} |