Ticket #4422 (closed defect: fixed)

Opened 5 weeks ago

Last modified 3 weeks ago

mc segfaults in extfs once archive contains file(s) in parent directory

Reported by: jnovy Owned by: andrew_b
Priority: major Milestone: 4.8.29
Component: mc-vfs Version: master
Keywords: Cc: mikhail.v.gavrilov@…
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

Please see https://bugzilla.redhat.com/show_bug.cgi?id=2158164 with 100% segfault reproducer.

Thanks!
Jindrich

Attachments

tmp.zip (488 bytes) - added by andrew_b 5 weeks ago.
example archive
2023-01-05 (7) (1).png (212.1 KB) - added by mikegav 5 weeks ago.
PowerZip-Screenshot.png (1.3 MB) - added by mikegav 5 weeks ago.
PowerZip-Screenshot.2.png (1.3 MB) - added by mikegav 5 weeks ago.

Change History

comment:1 Changed 5 weeks ago by andrew_b

  • Component changed from mc-core to mc-vfs

Changed 5 weeks ago by andrew_b

example archive

comment:2 Changed 5 weeks ago by andrew_b

  • Summary changed from mc segfaults in extfs.c once archive contains ".." at the beginning of file(s) to mc segfaults in extfs once archive contains file(s) in parent directory

comment:3 Changed 5 weeks ago by andrew_b

jnovy's patch helps partially: there is no segfault anymore but we can't enter to such archive.

comment:4 Changed 5 weeks ago by andrew_b

$ unzip -l tmp.zip 
Archive:  tmp.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2023-01-04 19:43   a
        0  2023-01-04 19:43   b
        0  2023-01-04 19:43   ../c
        0  2023-01-04 19:43   ../../d
---------                     -------
        0                     4 files

From point of VFS's view, archive has a root. If files are in the root and in child directories, everything is fine. VFS builds a tree with archive content successfully. But if archive contains files in the parent directory of archive root (../c and ../../d) we have a problem.

comment:5 Changed 5 weeks ago by zaytsev

  • Cc mikhail.v.gavrilov@… added

comment:6 Changed 5 weeks ago by zaytsev

Hack to avoid segfault:

diff -up mc-4.8.28/src/vfs/extfs/extfs.c.jnovy mc-4.8.28/src/vfs/extfs/extfs.c
--- mc-4.8.28/src/vfs/extfs/extfs.c.jnovy	2022-03-27 13:54:06.000000000 +0200
+++ mc-4.8.28/src/vfs/extfs/extfs.c	2023-01-04 15:55:08.081396288 +0100
@@ -254,8 +254,15 @@ extfs_find_entry_int (struct vfs_s_inode
         *q = '\0';
 
         if (DIR_IS_DOTDOT (p))
-            pent = pent->dir->ent;
-        else
+        {
+            if (pent->dir)
+            {
+              pent = pent->dir->ent;
+            } else
+            {
+              pent = NULL;
+            }
+        } else
         {
             GList *pl;

I wonder how other archivers like 7zip deal with this o_O

Changed 5 weeks ago by mikegav

comment:7 Changed 5 weeks ago by mikegav

7zip has separate button for up to real parent and '..' treated like a usual directory.

http://midnight-commander.org/raw-attachment/ticket/4422/2023-01-05%20(7)%20(1).png

Changed 5 weeks ago by mikegav

Changed 5 weeks ago by mikegav

comment:8 Changed 5 weeks ago by mikegav

Here is possible three solutions:
1) As it do 7z. It showing honestly '..' as directory and let enter. It best solution, but I suppose it hart to implement in mc because '..' reserved as level up.

2) As it do PowerZip? (MacOs?).
http://midnight-commander.org/raw-attachment/ticket/4422/PowerZip-Screenshot.png
It replace name '..' by '__Parent__' I suppose it can be implemented in mc. And it preferable solution because we able extract files behind '..' directory.

3) As to do Windows Explorer. It just ignore '..' and I can't extract files behind '..' directory without 3rd party software. It worst solution.

Last edited 5 weeks ago by mikegav (previous) (diff)

comment:9 Changed 5 weeks ago by andrew_b

Let's split this problem into two parts: 1) (this thicket): fix the segfault (possibly at the cost of not being able to enter the archive) and 2) (another ticket) support archives with files in the parent directory.

comment:10 follow-up: ↓ 11 Changed 5 weeks ago by mikegav

1) (this thicket): fix the segfault (possibly at the cost of not being able to enter the archive)

I would prefer just ignore '..' instead of not able to enter the archive

comment:11 in reply to: ↑ 10 Changed 5 weeks ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.29

Replying to mikegav:

I would prefer just ignore '..' instead of not able to enter the archive

#4424

Segfault fix:
branch:4422_extfs_segfault
Initial changeset:69ed5696f2ed47bd99ba1ca2c4257540f4a5c7ff

comment:12 Changed 4 weeks ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:13 Changed 4 weeks ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master: [ce16fa57462b338c3ee2984be4e8d3f026eb0444].

git log --pretty=oneline ba64c89b9..ce16fa574

comment:14 Changed 4 weeks ago by andrew_b

  • Status changed from testing to closed

comment:15 Changed 3 weeks ago by andrew_b

#4427: fixup: "Inconsistent archive" error for valid RPM.

Note: See TracTickets for help on using tickets.