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

OS specific invalid characters are causing extraction to corrupt file #858

Closed
DineshSolanki opened this issue Jul 24, 2024 · 7 comments
Closed

Comments

@DineshSolanki
Copy link
Contributor

https://www.deviantart.com/zenoasis/art/Japanese-TV-Dorama-folder-icon-pack-162-1077192465
Download zip from there
image

Extract using SharpCompress,
we'll see that the files REAL 恋愛殺人捜査班 : Real - Renai Satsujin Sosa Han.png and あの子の子ども : My Girlfriend's Child.png are extracted as 0 byte file with following names REAL µüïµä¢µ«║Σ║║µì£µƒ╗τÅ¡ , πüéπü«σ¡Éπü«σ¡Éπü¿πéÖπéé

I also tried with IBM437 encoding but same result.

However when you extract using 7zip you can see that it extracts fine and 7zip makes some changes to file name - which seeems to be removing : character which might be either U+A789 or U+2236

filenames from 7zip extraction REAL 恋愛殺人捜査班 _ Real - Renai Satsujin Sosa Han.png, あの子の子ども _ My Girlfriend's Child.png

@Morilli
Copy link
Contributor

Morilli commented Jul 24, 2024

There is no validation of the destination file name, so an attempt is made to write to a filestream with a filename containing :.
Interestingly this seems to somewhat work and no library function throws an exception (not even Path.GetFullPath even though it's clearly documented to do so), but the output is kinda garbage.

While I'm actually kind of interested in figuring out what actually happens when this is attempted, the fix here is most likely to sanitize the output path before attempting to open any output:

diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs
index 27d4164..80b8e9d 100644
--- a/src/SharpCompress/Common/ExtractionMethods.cs
+++ b/src/SharpCompress/Common/ExtractionMethods.cs
@@ -37,6 +37,7 @@ internal static class ExtractionMethods
         options ??= new ExtractionOptions() { Overwrite = true };

         var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")).NotNull("File is null");
+        file = string.Join("_", file.Split(Path.GetInvalidFileNameChars()));
         if (options.ExtractFullPath)
         {
             var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null"))

@DineshSolanki
Copy link
Contributor Author

DineshSolanki commented Jul 25, 2024

Yes that seems to be the solution, I wonder if 7zip handles it in same way.
to answer on what happens when we try it, it seems to truncate the filename and stream is consumed somewhere else as its definitely not writing to that file with truncated name.
even windows explorer can't extract it

image

@adamhathcock
Copy link
Owner

Could it be as simple as put the string through the encoding?

@DineshSolanki
Copy link
Contributor Author

DineshSolanki commented Jul 25, 2024

Could it be as simple as put the string through the encoding?

didn't work, tried UTF 8 and IBM437

var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")).NotNull("File is null");
var encoding = Encoding.UTF8;
 file = encoding.GetString(encoding.GetBytes(file)) ?? "";

However a 7zip discussion also mentions the encoding https://sourceforge.net/p/sevenzip/discussion/45798/thread/82ae0f9c/
Maybe I'm encoding in wrong manner?

@DineshSolanki
Copy link
Contributor Author

DineshSolanki commented Jul 25, 2024

issue seems to be with invalid character itself instead of the encoding, I was wrong in saying that the colon was probably unicode U+A789.

the following code will run into same issue

File.WriteAllText("あの子の子ども : My Girlfriend's Child.txt", "test");

so we have to remove invalid characters as @Morilli suggested, I tried his solution and it works

7zip https://github.com/ip7z/7zip/blob/a7a1d4a241492e81f659a920f7379c193593ebc6/CPP/7zip/UI/Common/ExtractingFilePath.cpp#L23

and
the SevenZipSharp also seems to does the same https://github.com/StevenBonePgh/SevenZipSharp/blob/d0bf5c4a3d65ea5e85ffe7ef94312354475f5714/SevenZip/ArchiveExtractCallback.cs#L624C13-L641C14

@DineshSolanki DineshSolanki changed the title unicode characters are causing file to Currupt at extraction OS specific invalid characters are causing extraction to corrupt file Jul 25, 2024
@adamhathcock
Copy link
Owner

Your fix names sense: we shouldn't put invalid path characters in....but seems like it wouldn't work for everything? I'm inclined to accept it as other things do it

@DineshSolanki
Copy link
Contributor Author

@adamhathcock It surely work for the issue we are having, its the same logic that 7zip is using

adamhathcock added a commit that referenced this issue Jul 30, 2024
…-in-filename

Fix #858 - Replaces invalid filename characters
AptiviCEO added a commit to Aptivi/Nitrocid that referenced this issue Sep 16, 2024
---

SharpCompress allowed files with invalid characters (according to Windows) to be extracted incorrectly, causing data loss. Therefore, fixing issue adamhathcock/sharpcompress#858 is important.

---

Type: upd
Breaking: False
Doc Required: False
Backport Required: False
Part: 1/1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants