[vlc-devel] [PATCH] codec/subsusf: prevent heap-use-after-free / leak

[Filip Roséen]

The memcpy used will of course make p_ssa_style and p_default_style
refer to the same text_style_t, resulting in a use-after-free during
clean-up (as well as a leak due to the original p_ssa_style->p_style
being lost).

--

ERROR: AddressSanitizer: heap-use-after-free on address 0x607000075570 at pc 0x7ffff68e0d3c bp 0x7fffd5950360 sp 0x7fffd5950350
READ of size 8 at 0x607000075570 thread T7
    #0 0x7ffff68e0d3b in text_style_Delete /home/refp/work/videolan/vlc/git/src/misc/text_style.c:152
    #1 0x7fffbdf11521 in CloseDecoder /home/refp/work/videolan/vlc/git/modules/codec/subsusf.c:188
    #2 0x7ffff67dc3e7 in generic_stop /home/refp/work/videolan/vlc/git/src/modules/modules.c:357
    #3 0x7ffff67dd809 in vlc_module_unload /home/refp/work/videolan/vlc/git/src/modules/modules.c:338
    #4 0x7ffff67dd8f9 in module_unneed /home/refp/work/videolan/vlc/git/src/modules/modules.c:371
    #5 0x7ffff681a6e5 in UnloadDecoder /home/refp/work/videolan/vlc/git/src/input/decoder.c:192
    #6 0x7ffff681eb99 in DeleteDecoder /home/refp/work/videolan/vlc/git/src/input/decoder.c:1740
    #7 0x7ffff6822e29 in input_DecoderDelete /home/refp/work/videolan/vlc/git/src/input/decoder.c:1939
    #8 0x7ffff6829baa in EsDestroyDecoder /home/refp/work/videolan/vlc/git/src/input/es_out.c:1690
    #9 0x7ffff682e01e in EsUnselect /home/refp/work/videolan/vlc/git/src/input/es_out.c:1801
    #10 0x7ffff6835eb8 in EsOutControlLocked /home/refp/work/videolan/vlc/git/src/input/es_out.c:2246
    #11 0x7ffff6835eb8 in EsOutControl /home/refp/work/videolan/vlc/git/src/input/es_out.c:2779
    #12 0x7ffff683fc57 in es_out_vaControl /home/refp/work/videolan/vlc/git/include/vlc_es_out.h:137
    #13 0x7ffff683fc57 in es_out_Control /home/refp/work/videolan/vlc/git/include/vlc_es_out.h:146
    #14 0x7ffff68403af in CmdExecuteControl /home/refp/work/videolan/vlc/git/src/input/es_out_timeshift.c:1550
    #15 0x7ffff68441c2 in ControlLocked /home/refp/work/videolan/vlc/git/src/input/es_out_timeshift.c:663
    #16 0x7ffff68441c2 in Control /home/refp/work/videolan/vlc/git/src/input/es_out_timeshift.c:768
    #17 0x7ffff6848d82 in es_out_vaControl /home/refp/work/videolan/vlc/git/include/vlc_es_out.h:137
    #18 0x7ffff6848d82 in es_out_Control /home/refp/work/videolan/vlc/git/include/vlc_es_out.h:146
    #19 0x7ffff684ac5a in es_out_SetMode /home/refp/work/videolan/vlc/git/src/input/es_out.h:89
    #20 0x7ffff684ac5a in End /home/refp/work/videolan/vlc/git/src/input/input.c:1440
    #21 0x7ffff6858eb8 in Run /home/refp/work/videolan/vlc/git/src/input/input.c:498
    #22 0x7ffff5993453 in start_thread (/usr/lib/libpthread.so.0+0x7453)
    #23 0x7ffff56d67de in __GI___clone (/usr/lib/libc.so.6+0xe87de)
---
 modules/codec/subsusf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/modules/codec/subsusf.c b/modules/codec/subsusf.c
index 8360db61b8..d8a9519e99 100644
--- a/modules/codec/subsusf.c
+++ b/modules/codec/subsusf.c
@@ -630,12 +630,17 @@ static void ParseUSFHeaderTags( decoder_t *p_dec, xml_reader_t *p_xml_reader )
                         if( !strcasecmp( p_sys->pp_ssa_styles[i]->psz_stylename, "Default" ) )
                         {
                             ssa_style_t *p_default_style = p_sys->pp_ssa_styles[i];
+                            text_style_t *p_orig_text_style = p_ssa_style->p_style;
 
                             memcpy( p_ssa_style, p_default_style, sizeof( ssa_style_t ) );
+
+                            // reset data-members that are not to be overwritten
+                            p_ssa_style->p_style = p_orig_text_style;
+                            p_ssa_style->psz_stylename = NULL;
+
                             //FIXME: Make font_style a pointer. Actually we double copy some data here,
                             //   we use text_style_Copy to avoid copying psz_fontname, though .
                             text_style_Copy( p_ssa_style->p_style, p_default_style->p_style );
-                            p_ssa_style->psz_stylename = NULL;
                         }
                     }
 
-- 
2.11.1