Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xar: Avoid infinite link loop #2123

Closed
wants to merge 1 commit into from

Conversation

stoeckmann
Copy link
Contributor

A file may have only one link target at a time. Otherwise the internal link structure could loop. Besides, a hard link realistically can only link to one file, not multiple ones.

Consider such an archive invalid.

Proof of Concept:

  1. Create archive which contains multiple files with multiple links
$ base64 -d > archive.xar << EOF
eGFyIQAcAAEAAAAAAAAB1gAAAAAAAAU8AAAAAXic7VTLcqMwELznKyjuRA9kLKdkcssXZC97E9KA
VeHhAtlF8vWRxMPrsr2pve+JmZ5Wa5jpkngdmzo6Qz+Yrt3H5BnHEbSq06at9vGv97eEx6/5kxhl
nz9FwnbKfSKhepDWnUisaSCnmLIEs4Tgd4pfNvxlwwS6poRDB1Afw6mJBvtZwz4eDpLEvhKJriwH
sDkWaI4COpgvLy5QCLwEWjRCVpoaIqNd27NMK91VHn22oxUopKFgP48Q1ab92MddbyrTyjrOD7LX
HhPIl/+RaNpOQ84Y54wLNGWhoOFsFLSda5ztBFrTUGw8DWfMzadZT5yMzgnG7j99NEED9LntCiMH
h/okwNXKrBZm1Xen40qdslBQ91dD2W83xWUnrqP7NEwcrbnQ5AO1raPJC01LK0MUiRrayh5ykgk0
hxM+L5he73peNr/s2kGLERfDyOOxNiq4Co1J9WWOMZqpslcHcwad3DcZ6LIgjDJebPSuTLMy1UBp
udvgXbpR2xKrIgVauF5vhJZeRttLZR/esC1hiznFgLkkUioMJaVM71SWySLLeEqUYrIgAt0qTbND
y/AE8ha+djj90+GLH//icvKjvR8w/vv61tfrQtyg/AMoUHgOvwGsBYwuAtBf07YHHCCVifUe8mGg
CXTEPed4nEvOzytJzSvhAgAO0QMGeJztVMtyozAQvOcrKO5ED2Qsp2Q=
EOF
  1. Try to list content
$ bsdtar -tf archive.xar

An infinite loop is entered with 100 % CPU usage which will never complete (and neither fail due to memory constraints etc.).

A file may have only one link target at a time. Otherwise the internal
link structure could loop. Besides, a hard link realistically can only
link to one file, not multiple ones.

Consider such an archive invalid.
@kientzle
Copy link
Contributor

Thanks! The fix looks reasonable to me, but I'd like to include a test as well. Can you explain how to re-create your test archive?

@stoeckmann
Copy link
Contributor Author

Thanks! The fix looks reasonable to me, but I'd like to include a test as well. Can you explain how to re-create your test archive?

I have created the archive by patching libarchive to create invalid xar files due to duplicate link entries:

diff --git a/libarchive/archive_write_set_format_xar.c b/libarchive/archive_write_set_format_xar.c
index 2cf655d..bd46f8e 100644
--- a/libarchive/archive_write_set_format_xar.c
+++ b/libarchive/archive_write_set_format_xar.c
@@ -1241,6 +1241,8 @@ make_file_entry(struct archive_write *a, xmlTextWriterPtr writer,
 			filetype = "file";
 		break;
 	}
+	r = xmlwrite_string_attr(a, writer, "type", filetype,
+	    filelink, linkto.s);
 	r = xmlwrite_string_attr(a, writer, "type", filetype,
 	    filelink, linkto.s);
 	archive_string_free(&linkto);

With such a modified bsdtar, the commands are:

touch file1
ln file1 file2
./bsdtar --format xar -cf archive.xar file1 file2

mmatuska added a commit to mmatuska/libarchive that referenced this pull request Apr 23, 2024
A file may have only one link target at a time. Otherwise the internal
link structure could loop. Besides, a hard link realistically can only
link to one file, not multiple ones.

Consider such an archive invalid.

Co-authored-by: Martin Matuska <martin@matuska.de>
mmatuska added a commit to mmatuska/libarchive that referenced this pull request Apr 23, 2024
A file may have only one link target at a time. Otherwise the internal
link structure could loop. Besides, a hard link realistically can only
link to one file, not multiple ones.

Consider such an archive invalid.

Co-authored-by: Martin Matuska <martin@matuska.de>
mmatuska added a commit to mmatuska/libarchive that referenced this pull request Apr 23, 2024
A file may have only one link target at a time. Otherwise the internal
link structure could loop. Besides, a hard link realistically can only
link to one file, not multiple ones.

Consider such an archive invalid.

Co-authored-by: Martin Matuska <martin@matuska.de>
mmatuska added a commit to mmatuska/libarchive that referenced this pull request Apr 23, 2024
A file may have only one link target at a time. Otherwise the internal
link structure could loop. Besides, a hard link realistically can only
link to one file, not multiple ones.

Consider such an archive invalid.

Co-authored-by: Martin Matuska <martin@matuska.de>
@mmatuska
Copy link
Member

Merged

@mmatuska mmatuska closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants