Sumatrapdf: [FIXED in Git 679c45f28] Unrar.dll - technical proposal for correct extraction and find for load algoritm used after Git-2ce8617ba

Created on 31 Dec 2018  路  14Comments  路  Source: sumatrapdfreader/sumatrapdf

Users make reasonable comments about the loss of portability of the program after Git 2ce8617ba - the unrar.dll library is always unpacked in the %LOCALAPPDATA%\SumatraPDF\Extracted\ directory even if it is in the same directory as the SumatraPDF executable.

To eliminate their comments, I propose to adjust the algorithm for searching and loading this DL like:

Proc DLLTools(); 
 Dcl CDir Char
  Dcl DllPatch Char
  CDir = GetEnv(%CurrentDir%);
      Begin; 
          IfFileExists(CDir\unrar.dll) Then Load(CDir\unrar.dll) Else
              Begin
                   If (GetFileAttrib(CDir) == "Read Only") or (GetDirLockStatus(CDir) == "Locked")) Then 
                      DllPatch = GetEnv(%TEMP%)..L'\SumatraPDF\Extracted Else DllPatch = CDir;
                   End;
                     Put(unrar.dll,DllPatch);
                     Load(unrar.dll,DllPatch)
               End;
            End;
        End; 
      Return;      
   End;

P.S.

_And taking this opportunity, let me congratulate you on the upcoming New Year and wish you happiness, health and more good and interesting people on the path of life._

Most helpful comment

Thanks for the good wishes and happy New Year to you as well.

Yes, I'll improve this to use existing dll first.

All 14 comments

Thanks for the good wishes and happy New Year to you as well.

Yes, I'll improve this to use existing dll first.

Thank you And once again you a Happy New Year!

I rewrites src\AppTools.cpp DLL extract algorithm and added try check for Read Only folders:

Rewrites src\AppTools.cpp DLL extract algorithm

/* Begin startup variable block */

Dcl ExePath Char
Dcl DllPath Char

/* Get random part for DLL patch */

   Suf = Subst(1,32,Random());

/* Get 'SumatraPDF.exe' patch use WinAPI */

   ExePath = GetExePath('SumatraPDF.exe');

/* End startup variable block */

/* Begin function block */

/* Check startup patch */

/* Check "%Program Files%" or "%Program Files(x86)%" */
  Function CheckPF()
     Return = (Subst(4,13,ExePath) == "Program Files");
  End;

/* End function block */

/* Begin SumatraPDF.exe DLL init block */

   If ((CheckPF() or IsAttrib(ExePath,ReadOnly)) Then
      Begin:
        If IsAmd64('SumatraPDF.exe') Then
          Put('unrar64.dll',GetEnv('APPDATA').."\Local\SumatraPDF\extracted");
         Else
          Put('unrar.dll',GetEnv('APPDATA').."\Local\SumatraPDF\extracted");
        End;
          DllPath = GetEnv('APPDATA').."\Local\SumatraPDF\extracted";
      End;
     Else
      Begin;
       If not (IsExist('unrar.dll',ExePath) or IsExist('unrar64.dll',ExePath)) Then
        If IsAmd64('SumatraPDF.exe') Then Put('unrar64.dll',ExePath) Else Put('unrar.dll',ExePath) End;
        DllPath = ExePath;
       End;
      End;
    End;

/* Load needed DLL */

    If IsAmd64 Then load('unrar64.dll',DllPath) Else load('unrar.dll',DllPath) End;

/* End SumatraPDF.exe DLL init block */

_Reasone for editing is minimized write's on Flash/SSD disks_

Test files for check open RAR 5.0 format - test-rar29.cbr - used RAR 2.9 format is supported the lib unaar and test-rar50.cbr used RAR 5.0 format (header signature 0x52 0x61 0x72 0x21 0x1a 0褏00 0x00) not supported existing version lib unaar.

test-rar29.cbr.zip
test-rar50.cbr.zip

I read Rar unpack code in to rar/rar rar.c (212):

    if (memcmp(signature, "Rar!\x1A\x07\x00", sizeof(signature)) != 0) {
        if (memcmp(signature, "Rar!\x1A\x07\x01", sizeof(signature)) == 0)
            warn("RAR 5 format isn't supported");

and in the yard the 2019th year :)

ps. - fix typo

I added wish up to unarr project for add support RAR5 format Issues #3 ...

While the 32 and 64-bit DLLs are extracted to different randomdirs so there's no overlap, the main issues are loss of both self-containment and stealth portability.

Loss of self-containment is obvious since the portable versions instead of extracting the UnRAR DLLs to the EXE's folder are extracting to %LocalAppData% by default. %LocalAppData% should IMO be used _only_ as a fallback in case the portable EXE's folder is not writeable for some reason.

Loss of stealth portability is due to the fact that closing neither the 32 or 64-bit portable version seems to wipe its associated randomdir (although the 64-bit version wipes the 32-bit one's randomdir and vice versa).

So not only are the current 'portable' versions not truly self-contained, they're not even cleaning up after themselves. 馃槥

Fixed in Git 679c45f28

@kjk: So if I understand the changes correctly, the latest builds are no longer extracting any UnRAR DLLs at all and are relying on incorporating the UnRAR source itself into Sumatra? And if so, does this mean UnArr is no longer being used?

While this seems fine and dandy, a matter of concern when it comes to relying on external sources is that if Sumatra experiences another prolonged development hiatus then in effect the UnRAR code inside (with bugs if any) will be 'frozen' too, despite the fact that newer UnRAR DLLs will continue to be released. External DLLs do have the advantage of allowing the user to simply substitute them with the latest API-compatible release...

Yes to all that.

However, in practice:

  • UnRAR code doesn't have bugs
  • almost no-one would actually bother to replace UnRAR.dll used by Sumatra. I doubt many people even knew it was an option

The format of the RAR container changes quite rarely (it changed in RAR 1.3, RAR 1.4, RAR 1.5, RAR 2.9 and RAR 5.0,) and the changes made to the UnRAR code are mostly service-based, which we see on WhatsNew.txt in ftp://Anonymous:[email protected]:@ftp.rarlabs.com:21/rar/UnRARDLL.exe, and the unrarsrc*.tar.gz sources are which can be download from ftp://Anonymous:[email protected]:@ftp.rarlabs.com:21/rar/unrarsrc-.tar.gz (current version 5.8.4 is the third digit is the current beta number), and the library itself has not changed much since 2013 (source UnRARDLL.exe/WhatsNew.txt, extract):

  • February 2, 2019

New CmtBufW field of RAROpenArchiveDataEx structure allows to read the archive comment in Unicode.

  • 10 January 2019

New OpFlags field and ROADOF_KEEPBROKEN flag in RAROpenArchiveDataEx structure to specify if extracted files with invalid checksum shall be preserved.
Reserved [28] in the same structure is changed to Reserved [27].

  • 3 October 2018

UnpVer field of RARHeaderData and RARHeaderDataEx structures erroneously returned 200 instead of 50 for RAR 5.0 archives. Fixed now.

  • 17 September 2016

New MtimeLow, MtimeHigh, CtimeLow, CtimeHigh, AtimeLow, AtimeHigh fields of RARHeaderDataEx structure provide information about high precision times of archived files.

RAR_DLL_VERSION is incremented to 8.

  • July 6, 2015

New RedirType, RedirName, RedirNameSize, DirTarget fields of RARHeaderDataEx structure provide information about file system redirection headers.

Please be sure to fill either the entire RARHeaderDataEx structure or at least its Reserved field with zeroes before passing to RARReadHeaderEx.

RAR_DLL_VERSION is incremented to 7.

  • November 5, 2013

OpenResult field of RAROpenArchiveData and RAROpenArchiveDataEx structures can return the new ERAR_BAD_PASSWORD error code.
This code can be returned only for archives with encrypted headers in RAR 5.0 format.

so I don鈥檛 think that we can encounter a hidden error in the code of the RAR5 unpacker, and if Yevgeny Roshal changes the format of the container, he will immediately lay out his new description and the source code of the new UnRAR and we can take this into account.

I really wouldn't go so far as to declare with absolute certainty that the UnRAR code has _no_ bugs or vulnerabilities whatsoever, but anyway, as long as any significant changes are incorporated into Sumatra on a timely basis I guess it's all good. :) Certainly one less thing for your average user who knows nothing of DLLs to contend with.

Eugene is often the subject of WinRAR on forun.ru-board.com and I know how carefully he checks any suspicion of errors - they have practically no chance to go unnoticed, including the people who hunt them. And even an error occurs, until the next update of the unrar sources in the DLL / EXE is saved, and if it is easier to fix it ourselves and transfer the patch to Eugene, for which he will only be grateful. At least during the period of our acquaintance with him, I had an opinion about him as a very friendly, decent, and attentive person.

@VictorVG: Eugene is definitely polite and a conscientious programmer. However no disrespect to him, but the very nature of the beast is such that _any_ complex piece of software is bound to have bugs and vulnerabilities, and even open source (which WinRAR except for the UnRAR part isn't) cannot guarantee perfection. In short, let's not pretend that his programs have never suffered from any bugs at all (even long-term ones), and if you need proof you can check out various exploit/vulnerability DBs including 1, 2, 3 and 4 (not to mention those on the dark web where exploits are regularly bought and sold).

Anyway, as I've said already above, we have nothing to worry about as long as Sumatra is under active maintenance and development and incorporates updated (bug-fixed) third-party components on a timely basis.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

yozhic picture yozhic  路  5Comments

lanhktc picture lanhktc  路  3Comments

Gugus99 picture Gugus99  路  3Comments

vvRFvv picture vvRFvv  路  4Comments

gblazex picture gblazex  路  7Comments