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

Add bounds checks to rfc5424 parser #62

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

libc
Copy link
Contributor

@libc libc commented Mar 15, 2019

Hello,

We're streaming logs via syslog from our CDN provider, and sometimes the server crashes with "out of range panic":

panic: runtime error: index out of range

goroutine 243 [running]:
gopkg.in/mcuadros/go-syslog.v2/internal/syslogparser/rfc5424.(*Parser).parseTimestamp(0xc000704c00, 0x1, 0x0, 0x0, 0x0, 0x0)
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/internal/syslogparser/rfc5424/rfc5424.go:196 +0x39d
gopkg.in/mcuadros/go-syslog.v2/internal/syslogparser/rfc5424.(*Parser).parseHeader(0xc000704c00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/internal/syslogparser/rfc5424/rfc5424.go:141 +0x107
gopkg.in/mcuadros/go-syslog.v2/internal/syslogparser/rfc5424.(*Parser).Parse(0xc000704c00, 0xc000b58900, 0xc000704c00)
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/internal/syslogparser/rfc5424/rfc5424.go:85 +0x71
gopkg.in/mcuadros/go-syslog%2ev2.(*Server).parser(0xc00014e580, 0xc00080e5a8, 0x7, 0x8, 0xc0002a0be0, 0x10, 0x0, 0x0)
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/server.go:254 +0x8e
gopkg.in/mcuadros/go-syslog%2ev2.(*Server).scan(0xc00014e580, 0xc0003825c0, 0xc0002a0be0, 0x10, 0x0, 0x0)
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/server.go:242 +0xd8
created by gopkg.in/mcuadros/go-syslog%2ev2.(*Server).goScanConnection
        /go/pkg/mod/gopkg.in/mcuadros/go-syslog.v2@v2.2.1/server.go:227 +0x272

It could be caused by our network, not CDN, but in any case we would like to prevent crashing and gracefully error out.

I fixed the panic mentioned above with an explicit test TestParseTimestamp_Empty. Then I added a test that feeds truncated messages to the panic and prevented corresponding panics.

Please let me know if anything needs to be changed.

Several bounds checks are missing from rfc5424 parser, which can cause
"index out of range" panic.

We see this index out of range panics in our production environment
while streaming logs from a 3rd party CDN server.
@mcuadros mcuadros merged commit 277e703 into mcuadros:master Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants