viewer: fix crash when viewing files with PK/ZIP magic bytes#5058
Open
ilia-maslakov wants to merge 3 commits intoMidnightCommander:masterfrom
Open
viewer: fix crash when viewing files with PK/ZIP magic bytes#5058ilia-maslakov wants to merge 3 commits intoMidnightCommander:masterfrom
ilia-maslakov wants to merge 3 commits intoMidnightCommander:masterfrom
Conversation
github-actions
bot
added
needs triage
2 tasks
Contributor
|
/rebase |
When mcview opens a file with PK/ZIP magic bytes (e.g. PPTX, DOCX, ODS, ODT, JAR), get_compression_type() detects COMPRESSION_ZIP and attempts VFS decompression. If mc_open() on the VFS path fails (fd1 == -1): 1. mcview_close_datasource(view) is called but datasource is DS_NONE (no-op) 2. mcview_show_error() for in-panel viewers calls mcview_set_datasource_string(), setting datasource to DS_STRING 3. No goto/return, so execution falls through to mcview_set_datasource_file() which overwrites datasource to DS_FILE without cleaning up DS_STRING state This corrupts viewer state and causes segfault on subsequent F3 invocation. Fix: when VFS decompression fails, silently fall through to display raw file content. The file is valid and readable, there is no reason to show an error. Closes: MidnightCommander#4760 Signed-off-by: Ilia Maslakov <il.smind@gmail.com>
Add tests verifying that mcview_load correctly handles files with PK/ZIP magic bytes (PPTX, DOCX, ODS, JAR, etc.) when VFS decompression fails. Tests cover: - ZIP-magic file loads as DS_FILE without error - repeated load does not crash (regression for second-F3 segfault) - normal text file loads correctly - nonexistent file returns FALSE - gzip-magic file also loads without error Mark viewer display/lib functions and file_error_message, message, load_file_position as MC_MOCKABLE to enable test overrides. Signed-off-by: Ilia Maslakov <il.smind@gmail.com>
Add extern forward declarations for global linker stub variables (mc_skin_color__cache, mc_editor_plugin_list, we_are_strstrstrbackground) to satisfy -Wmissing-variable-declarations. These variables must remain non-static as they are referenced by name from libinternal.a objects. Signed-off-by: Ilia Maslakov <il.smind@gmail.com>
mc-butler
force-pushed
the
fix-viewer-zip-crash
branch
from
6221ecb to
74c9f0c
Compare
mc-worker
added
area: mcview
Contributor
|
@ilia-maslakov I've cleaned up your test and created a |
Member
I vaguely remember we had a related problem some time ago: What I'm not sure about is what the right way to behave here is. I think the idea before was to show an error message. Now the file is silently opened in the raw mode. Is this desired @mc-worker ? Do you know if this is consistent with other viewer behaviors? |
zyv
requested changes
Apr 5, 2026
| void mc_editor_plugins_load (void) {} | ||
| int button_get_width (void *b) { (void) b; return 0; } | ||
|
|
||
| #pragma GCC diagnostic pop |
Member
There was a problem hiding this comment.
I think this should be handled as it is done here: b6012a2 .
Comment on lines
+67
to
+70
| mcview_compute_areas (WView *view) | ||
| { | ||
| (void) view; | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| mcview_compute_areas (WView *view) | |
| { | |
| (void) view; | |
| } | |
| mcview_compute_areas (MC_UNUSED WView *view) | |
| { | |
| } |
Comment on lines
+151
to
+152
| ssize_t written = write (fd, data, size); | ||
| (void) written; |
Member
There was a problem hiding this comment.
Suggested change
| ssize_t written = write (fd, data, size); | |
| (void) written; | |
| write (fd, data, size); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
In
mcview_load(), whenget_compression_type()detects a ZIP header andmc_open()on the VFS decompression path fails (fd1 == -1):mcview_close_datasource(view)is called but datasource isDS_NONE(no-op)mcview_show_error()for in-panel viewers callsmcview_set_datasource_string(), setting datasource toDS_STRINGgoto/return, so execution falls through tomcview_set_datasource_file()which overwrites datasource toDS_FILEwithout cleaning upDS_STRINGstateSample content of the test file
The following hex content produces a file that causes
mcviewto segfault when opened:50 4B 03 04 14 00 00 00 08 00This corrupts viewer state and causes segfault on subsequent F3.
Test plan