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

Avoid double-free in Analysisd's decoder for Windows Eventchannel #3626

Merged
merged 1 commit into from Jul 16, 2019

Conversation

vikman90
Copy link
Member

@vikman90 vikman90 commented Jul 6, 2019

Related issue
#3618

Messages from Windows Eventchannel (as of Wazuh 3.8) contain XML data. Analysisd parses it as part of the decoding stage.

This decoder is clearing (freeing) the XML structure either the parsing was successful or not. However, according to the rest of the components that parse XML, the structure must only be cleaned in case of success.

This may produce a double-free condition.

AddressSanitizer report

Forcing an XML parsing error with a randomly initialized XML structure:

    #0 0x55f0997b6cff in OS_ClearXML os_xml/os_xml.c:92
    #1 0x55f099668f6f in DecodeWinevt analysisd/decoders/winevtchannel.c:813
    #2 0x55f099646a4f in w_decode_winevt_thread analysisd/analysisd.c:2167
    #3 0x7f083ff3afa2 in start_thread /build/glibc-vjB4T1/glibc-2.28/nptl/pthread_create.c:486
    #4 0x7f083fe6b4ce in clone (/lib/x86_64-linux-gnu/libc.so.6+0xf94ce)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV os_xml/os_xml.c:92 in OS_ClearXML
Thread T52 created by T0 here:
    #0 0x7f0840589db0 in __interceptor_pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x50db0)
    #1 0x55f099768dac in CreateThreadJoinable shared/pthreads_op.c:47
    #2 0x55f099768f31 in CreateThread shared/pthreads_op.c:62
    #3 0x55f099640489 in OS_ReadMSG analysisd/analysisd.c:947
    #4 0x55f09963f6a5 in main analysisd/analysisd.c:711
    #5 0x7f083fd9609a in __libc_start_main ../csu/libc-start.c:308

==8095==ABORTING

Tests

  • Compile manager on Linux.
  • Source installation.
  • AddressSanitizer report (shown above).

@vikman90 vikman90 added type/bug Something isn't working module/analysis Issues related to the Analysis daemon labels Jul 6, 2019
@vikman90 vikman90 requested a review from snaow July 6, 2019 11:43
@vikman90 vikman90 self-assigned this Jul 6, 2019
@vikman90 vikman90 requested a review from cristgl July 8, 2019 07:28
@vikman90 vikman90 added this to In progress in Wazuh 3.9.4 via automation Jul 8, 2019
@vikman90 vikman90 modified the milestone: 28th week Jul 8, 2019
@vikman90 vikman90 removed this from In progress in Wazuh 3.9.4 Jul 8, 2019
@vikman90 vikman90 merged commit 71ec28a into 3.9 Jul 16, 2019
@vikman90 vikman90 deleted the 3.9-fix-winevt-free branch July 16, 2019 15:08
@chemamartinez chemamartinez added this to To do in Review 3.9.4 via automation Jul 25, 2019
@chemamartinez chemamartinez moved this from To do to Done in Review 3.9.4 Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/analysis Issues related to the Analysis daemon type/bug Something isn't working
Projects
No open projects
Review 3.9.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants