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

Fix handling of empty strings in UTF16BytesToString. #3705

Conversation

ohadravid
Copy link
Contributor

Hi,

I encountered a bug while using winlogbeat on Windows Server 2003.
Some events had errors in them:

{'@timestamp': '2017-03-01T08:53:18.000Z',
 '@version': '1',
 'beat': {'hostname': '...', 'name': '..', 'version': '5.2.1'},
 'computer_name': '...',
 'event_id': 540,
 'host': '...',
 'level': 'Audit Success',
 'log_name': 'Security',
 'message_error': 'Slice must have an even length (length=141)',
 'record_number': '1209',
 'source_name': 'Security',
 'tags': ['beats_input_raw_event'],
 'type': 'eventlogging',
 'user': {'...'}}

It seems to be a bug in the UTF16BytesToString function.
The function does not detect empty strings correctly (due to an off-by-one check on the return value of indexNullTerminator), and it creates a misaligned offset in the buffer.

I added a test and fixed it.

For completeness, this is an example of a 'bad' record I had (encoded in hex):
140200004c664c6502000000ff95b658ff95b65876170080040007000000000000000000600000000000000060000000700100009e0000004500760065006e0074004c006f0067000000570049004e0032004b00330052003200450045000000000000000000000031003700350000003600300000003000200047004d00540020005300740061006e0064006100720064002000540069006d006500000031002e0031000000300000004d006900630072006f0073006f00660074002000570069006e0064006f0077007300200053006500720076006500720020003200300030003300200052003200000035002e0032002e00330037003900300020004200750069006c006400200033003700390030002000530065007200760069006300650020005000610063006b0020003200000055006e006900700072006f0063006500730073006f00720020004600720065006500000033003700390030002e00730072007600300033005f007300700032005f006700640072002e003000390030003800300035002d00310034003300380000003400620034003600330063003400350000004e006f007400200041007600610069006c00610062006c00650000004e006f007400200041007600610069006c00610062006c0065000000300000003100000032003500360000003400300039000000770069006e0032006b003300720032006500650000000000000014020000

@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@andrewkroh
Copy link
Member

Thanks @ohadravid. I love it when bug reports come with fixes and tests. 👍

It looks like this bug affects v5.2.0, v5.2.1, v5.2.2+.

@andrewkroh
Copy link
Member

@ohadravid Could you please add an entry into the CHANGELOG.assciidoc file for this fix.

@andrewkroh
Copy link
Member

andrewkroh commented Mar 1, 2017

@ohadravid According the travis build the code needs gofmt'ed. Can you please run make fmt on in.

@andrewkroh andrewkroh merged commit 65b9385 into elastic:master Mar 2, 2017
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Mar 2, 2017
* Fix handling of empty strings in UTF16BytesToString.

(cherry picked from commit 65b9385)
andrewkroh added a commit that referenced this pull request Mar 2, 2017
* Fix handling of empty strings in UTF16BytesToString.

(cherry picked from commit 65b9385)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants