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

app-layer-ssl: fix memleak #1934

Closed
wants to merge 1 commit into from
Closed

Conversation

thus
Copy link
Contributor

@thus thus commented Mar 15, 2016

Avoid that the SNI extension code is executed multiple times on malformed packets, causing memory leaks.

https://redmine.openinfosecfoundation.org/issues/1736

Avoid that the SNI extension code is executed twice sometimes, causing
memory leaks.
@thus thus mentioned this pull request Mar 15, 2016
@@ -65,6 +65,9 @@ SCEnumCharMap tls_decoder_event_table[ ] = {
{ "INVALID_HEARTBEAT_MESSAGE", TLS_DECODER_EVENT_INVALID_HEARTBEAT },
{ "OVERFLOW_HEARTBEAT_MESSAGE", TLS_DECODER_EVENT_OVERFLOW_HEARTBEAT },
{ "DATALEAK_HEARTBEAT_MISMATCH", TLS_DECODER_EVENT_DATALEAK_HEARTBEAT_MISMATCH },
{ "MULTIPLE SNI EXTENSIONS", TLS_DECODER_EVENT_MULTIPLE_SNI_EXTENSIONS },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should not contain spaces but underscores. They are used as 'tls.multiple_sni_extensions' in the app-layer-events rule keyword.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add rules to rules/tls-events.rules for each of the new events?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: coffee first, then pull requests :)

Thanks for the comments. I'll fix!

@thus
Copy link
Contributor Author

thus commented Mar 15, 2016

Replaced by #1935

@thus thus closed this Mar 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants