Bug 10140

Summary:

The output is strange when using unzip to extract zip file

Product:

[Build System, Metadata & Runtime] OE-Core

Reporter:

Yi Zhao <yi.zhao>

Component:

core

Assignee:

Plauchu Edwin <edwin.plauchu.camacho>

Status:

VERIFIED DUPLICATE

QA Contact:

Yi Zhao <yi.zhao>

Severity:

normal

Priority:

Medium+

CC:

benjamin.esquivel, edwin.plauchu.camacho, jose.perez.carranza, meta.mr.watcher, meta.watcher, sgw

Version:

2.2

Target Milestone:

2.2 M3

Hardware:

x86

OS:

Multiple

Whiteboard:

OS type for building Yocto:

---

Type of Regression:

Regression (Used to work)

Verified:

Documentation change:

No (bug/feature does not impact docs)

Attachments:

Description	Flags
zip file for testing	none

Description Yi Zhao 2016-08-15 02:28:36 UTC

Created attachment 3355 [details]
zip file for testing

Git rev: master/e274a5167ea2e495355bd1382b1388d047b845d7

When using unzip to extract a zip file, the output is very strange:  
#########################################
# unzip tst_unzip_file.zip 
Archive:  %s
   creating: tst_unzip.dir/
   creating: tst_unzip.dir/d.0/
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
   creating: tst_unzip.dir/d.0/d.1/
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
   creating: tst_unzip.dir/d.0/d.1/d.2/
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
%8sing: %-22s  %s%s
#########################################

I did a quick check and found this issue was introduced by the following commit:
commit 763a3d424bccf559a8d6add3dc1f2746c82f2933
Author: Edwin Plauchu <edwin.plauchu.camacho@intel.com>
Date:   Fri May 27 15:29:21 2016 -0500

    unzip: fix security issues
    
    This patch avoids unzip fails to compile with compiler flags which elevate common string formatting issues into an error (-Wformat -Wformat-security -Werror=format-security).
    
    [YOCTO #9551]
    
    (From OE-Core rev: 2dd1c02fbc7492002df9030f50710e242369e8b2)
    
After I revert this patch, the output looks correct:
#########################################
# unzip tst_unzip_file.zip 
Archive:  tst_unzip_file.zip
   creating: tst_unzip.dir/
   creating: tst_unzip.dir/d.0/
 extracting: tst_unzip.dir/d.0/f.2   
 extracting: tst_unzip.dir/d.0/f.1   
   creating: tst_unzip.dir/d.0/d.1/
 extracting: tst_unzip.dir/d.0/d.1/f.2  
 extracting: tst_unzip.dir/d.0/d.1/f.1  
 extracting: tst_unzip.dir/d.0/d.1/f.0  
   creating: tst_unzip.dir/d.0/d.1/d.2/
 extracting: tst_unzip.dir/d.0/d.1/d.2/f.2  
 extracting: tst_unzip.dir/d.0/d.1/d.2/f.1  
 extracting: tst_unzip.dir/d.0/d.1/d.2/f.0  
 extracting: tst_unzip.dir/d.0/f.0
#########################################  

The zip file for testing is attached.

Comment 1 Ross Burton 2016-08-15 09:35:29 UTC

Jussi, can team FI fix the broken security fix?

Comment 2 Jussi Kukkonen 2016-08-17 08:07:50 UTC

Wow, the unzip macros are truly special... but I don't think the approach can be made to work:

+-#    define LoadFarString(x)        (char *)(x)
++#    define LoadFarString(x)        "%s",(char *)(x)

LoadFarString is used in 1500 locations, most of them in situations where the sprintf format string is already used. Example:

    Info(slide, 0x201, ((char *)slide,
         LoadFarString(SymLnkWarnInvalid), FnFilter1(linkfname)))

with this Info implementation

    define Info(buf,flag,sprf_arg) 
         fputs((char *)(sprintf sprf_arg, (buf)), stdout);

ends up calling sprintf like this:

    sprintf ((char *)slide,
             LoadFarString(SymLnkWarnInvalid),
             FnFilter1(linkfname))

Changing LoadFarSting() definition like in the patch will end up printing literal "%s" as filename.

Comment 3 Benjamin Esquivel 2016-08-18 15:11:42 UTC

bug 9551 got reopened to fix this problem.

*** This bug has been marked as a duplicate of bug 9551 ***

Comment 4 Jose Perez C 2016-09-09 16:04:21 UTC

verified as duplicate