Project

General

Profile

Actions

Bug #1091

closed

TLS-Handshake: Uninitialized value

Added by Jack Flemming about 10 years ago. Updated about 10 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

Version: Pulled from Github 1/25/2014. Advertised version is "Suricata 2.0dev (rev a77b9b3)"
Issue: Valgrind warns about an uninitialized value being used in app-layer-tls-handshake.c

==24210== Conditional jump or move depends on uninitialised value(s)
==24210==    at 0x45E887: TLSCertificateErrCodeToWarning (app-layer-tls-handshake.c:59)
==24210==    by 0x461966: DecodeTLSHandshakeServerCertificate (app-layer-tls-handshake.c:123)
==24210==    by 0x453757: SSLv3ParseHandshakeType (app-layer-ssl.c:185)
==24210==    by 0x455C6A: SSLv3ParseHandshakeProtocol (app-layer-ssl.c:312)
==24210==    by 0x458CC1: SSLv3Decode (app-layer-ssl.c:738)
==24210==    by 0x45CEEC: SSLDecode (app-layer-ssl.c:863)
==24210==    by 0x45D951: SSLParseServerRecord (app-layer-ssl.c:945)
==24210==    by 0x44418A: AppLayerParserParse (app-layer-parser.c:778)
==24210==    by 0x415D56: AppLayerHandleTCPData (app-layer.c:288)
==24210==    by 0x5AB418: StreamTcpReassembleAppLayer (stream-tcp-reassemble.c:3027)
==24210==    by 0x5ABCB7: StreamTcpReassembleHandleSegmentUpdateACK (stream-tcp-reassemble.c:3373)
==24210==    by 0x5ABD59: StreamTcpReassembleHandleSegment (stream-tcp-reassemble.c:3401)

How to recreate:
1. Make an unoptimized build of HTP / Suricata
2. Run like so:

valgrind --leak-check=full --trace-children=yes ./src/suricata -c ./suricata.yaml -r <attached pcap> -k none --runmode single -l ./output/

Patch:
Ultimately, the issue is that DecodeDer (util-decode-der.c) rejects the passed in data because it doesn't looks like ASN.1. However, it neglects to set the error code that we've passed in. It is unclear to me if it is the fault of app-layer-tls-handshake for not initializing the error code or if decode-der is always expected to populate the variable. Either way, the attached patch makes the issue go away.


Files

tls_warning.pcap (4.38 KB) tls_warning.pcap Jack Flemming, 01/27/2014 04:30 PM
decode_der.patch (758 Bytes) decode_der.patch Jack Flemming, 01/27/2014 04:31 PM
Actions #1

Updated by Victor Julien about 10 years ago

  • Target version changed from 2.0rc1 to 2.0rc2
Actions #2

Updated by Victor Julien about 10 years ago

  • Assignee set to Victor Julien
Actions #3

Updated by Victor Julien about 10 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF