madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.69k stars 2.45k forks source link

Do not store negative values on unsigned int in deflate.c #358

Open scarabeusiv opened 6 years ago

scarabeusiv commented 6 years ago

Simple patch to not crash when storing negative values in unsigned int.

diff --git a/deflate.c b/deflate.c
index 1ec7614..1bad1eb 100644
--- a/deflate.c
+++ b/deflate.c
@@ -1536,7 +1536,10 @@ local void fill_window(s)

         /* Initialize the hash value now that we have some input: */
         if (s->lookahead + s->insert >= MIN_MATCH) {
-            uInt str = s->strstart - s->insert;
+            uInt str = 0;
+            /* storing negative values to uInt is not good idea */
+            if (s->strstart - s->insert > 0)
+                str = s->strstart - s->insert;
             s->ins_h = s->window[str];
             UPDATE_HASH(s, s->ins_h, s->window[str + 1]);
 #if MIN_MATCH != 3
-- 
2.15.1
philippeVerney commented 4 years ago

I am also facing this same problem : negative values in str uInt value which crashes my app. I am on centos7 with zlib 1.2.7-18. I did not notice this bug on redhat6 and neither on redhat7 with zlib1.2.7-17.

This fill_window method is called by a minizip call trying to add a 895 bytes long string as a file in the zip. Here is my stack image

Here are my variable values in fill_window method where we can obviously see that s->strstart==0 and s->insert==8. This leads to a crash when trying to access s->window[str].

s   deflate_state * 0x7fbe6442a2e0  
    strm    z_streamp   0x7fbe6442c1e8  
        next_in Bytef * 0x7fbe6442a2d7 ""   
        avail_in    uInt    0   
        total_in    uLong   895 
        next_out    Bytef * 0x7fbe6442c298 "Ðpa\r"  
        avail_out   uInt    65536   
        total_out   uLong   0   
        msg char *  0x0 
        state   struct internal_state * 0x7fbe6442a2e0  
        zalloc  alloc_func  0x7fbe15ea4500 <zcalloc>    
        zfree   free_func   0x7fbe15ea4510 <zcfree> 
        opaque  voidpf  0x0 
        data_type   int 2   
        adler   uLong   1   
        reserved    uLong   2191057896  
    status  int 113 
    pending_buf Bytef * 0x7fbe643eb880 "(\b"    
    pending_buf_size    ulg 65536   
    pending_out Bytef * 0x7fbe643eb880 "(\b"    
    pending uInt    0   
    wrap    int 0   
    gzhead  gz_headerp  0x0 
    gzindex uInt    1701667186  
    method  Byte    8 '\b'  
    last_flush  int 0   
    w_size  uInt    32768   
    w_bits  uInt    15  
    w_mask  uInt    32767   
    window  Bytef * 0x7fbe6443c320 "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<eml20:EpcExternalPartReference xmlns:eml20=\"http://www.energistics.org/energyml/data/commonv2\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" schemaVersi"...   
    window_size ulg 65536   
    prev    Posf *  0x7fbe6444c330  
    head    Posf *  0x7fbe643db870  
    ins_h   uInt    0   
    hash_size   uInt    32768   
    hash_bits   uInt    15  
    hash_mask   uInt    32767   
    hash_shift  uInt    5   
    block_start long    0   
    match_length    uInt    2   
    prev_match  IPos    1701667186  
    match_available int 0   
    strstart    uInt    0   
    match_start uInt    1769353061  
    lookahead   uInt    895 
    prev_length uInt    2   
    max_chain_length    uInt    128 
    max_lazy_match  uInt    16  
    level   int 6   
    strategy    int 0   
    good_match  uInt    8   
    nice_match  int 128 
    dyn_ltree   struct ct_data_s [573]  0x7fbe6442a3a4  
    dyn_dtree   struct ct_data_s [61]   0x7fbe6442ac98  
    bl_tree struct ct_data_s [39]   0x7fbe6442ad8c  
    l_desc  struct tree_desc_s  {...}   
    d_desc  struct tree_desc_s  {...}   
    bl_desc struct tree_desc_s  {...}   
    bl_count    ush [16]    0x7fbe6442ae70  
    heap    int [573]   0x7fbe6442ae90  
    heap_len    int 805388800   
    heap_max    int 151007489   
    depth   uch [573]   0x7fbe6442b78c  
    l_buf   uchf *  0x7fbe643f7880 "\234l\203½eôaAhA(ï\223§@7©hTñú&A]1#\r]ôaAÌÓ¹¢t®§@Ã\232ÊÒìù&AmÊ\025C`ôaAq\e\ràÝ\024§@¡H÷s\215ù&A\a$ag]ôaA\003]û\002Ò˦@\"QhÑÞù&Aº¾\217³ZôaAé\236u\215îî¦@)éapÖú&A¥\205Ë/vôaAVb\236\225|\"¨@\221¹2xqú&Aù\017i¢rôaAUÞ\216pâÓ§@R(\vg¼ú&Aßú0&qôaA=\016\203ù\233õ§@'OY}¶ú&A\232\v\\GlôaA"... 
    lit_bufsize uInt    16384   
    last_lit    uInt    0   
    d_buf   ushf *  0x7fbe643ef880  
    opt_len ulg 0   
    static_len  ulg 0   
    matches uInt    0   
    insert  uInt    8   
    bi_buf  ush 0   
    bi_valid    int 0   
    high_water  ulg 0   
s@entry deflate_state * 0x7fbe6442a2e0  
str unsigned int    4294967288  
n   unsigned int    <optimized out> 
m   unsigned int    <optimized out> 
p   Posf *  <optimized out> 
more    unsigned int    <optimized out> 
wsize   unsigned int    32768   
madler commented 4 years ago

@philippeVerney Thank you for the report. Can you provide the code and data you used to aid in reproducing the issue?

philippeVerney commented 4 years ago

I am really sorry but not quickly at all. I am working on a totally big/huge proprietary application and for now I cannot reproduce the bug on a simple example. And of course I cannot share the big/huge proprietary application.

I really hope to be able to but this needs to be prioritized by my management..

For now, we are wondering if the RELRO support of centos7 zlib is not the problem : https://centos.pkgs.org/7/centos-x86_64/zlib-1.2.7-18.el7.x86_64.rpm.html Because we don't face the bug with the patch 17 of the zlib-1.2.7 on red hat 7 with the exact same code. This could explain the seg fault we get. But the problem would still exist and be silent.

However, I can easily share the string that I try to write as a file in the zip using minizip with some "SECRET" chars. It is

<?xml version="1.0" encoding="UTF-8"?>
<eml20:EpcExternalPartReference xmlns:eml20="http://www.energistics.org/energyml/data/commonv2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" schemaVersion="2.0" uuid="7ebbea72-6396-4047-af83-9889a28eb435" xsi:type="eml20:obj_EpcExternalPartReference">
    <eml20:Citation xsi:type="eml20:Citation">
        <eml20:Title xsi:type="eml20:DescriptionString">HDF5 proxy</eml20:Title>
        <eml20:Originator xsi:type="eml20:NameString">*SECRET*</eml20:Originator>
        <eml20:Creation xmlns:xsd="http://www.w3.org/2001/XMLSchema" xsi:type="xsd:dateTime">2020-06-12T14:47:29Z</eml20:Creation>
        <eml20:Format xsi:type="eml20:DescriptionString">[SECRE:SECRETe-SEC DAILY - 20200529]</eml20:Format>
    </eml20:Citation>
    <eml20:MimeType xmlns:xsd="http://www.w3.org/2001/XMLSchema" xsi:type="xsd:string">application/x-hdf5</eml20:MimeType>
</eml20:EpcExternalPartReference>
philippeVerney commented 4 years ago

I can also share part of the code.

The string is generated line 163 of https://github.com/F2I-Consulting/fesapi/blob/v1.2.0.0/src/common/EpcDocument.cpp Then it goes into l.427 of https://github.com/F2I-Consulting/fesapi/blob/v1.2.0.0/src/epc/Package.cpp then l.484 And finally the l.498 crashes...

But I don't think it helps a lot.. I will post a more didactic example if I can find time for it and if I can do it.

philippeVerney commented 4 years ago

I think to have found the problem and it is not cause of zlib.

The big application I am working one loads libQtCore which embeds a zlib version v1.2.3 My plugin links to system zlib v1.2.7 Both zlib were in conflict at application runtime.

Regards