Panic in rfc3164.go L192 about syslogparser HOT 4 CLOSED

I originally thought that this might happen on a blank message, but I think it happens when the message itself is just one word long, as it tries to parse it as a tag and panics when it doesn't find the end and keeps incrementing the cursor.

I went the naive route and added a load of Printf statements to parseTag(). Here's two successful parses and a failure:

myapp[5394]: in parsetag cursor 32 - len 38
myapp[5394]: line: <13>Mar 18 12:32:25 log-output: 200 OK
myapp[5394]: in loop p.cursor: 32
myapp[5394]: doing byte: 2
myapp[5394]: in loop p.cursor: 33
myapp[5394]: doing byte: 0
myapp[5394]: in loop p.cursor: 34
myapp[5394]: doing byte: 0
myapp[5394]: in loop p.cursor: 35
myapp[5394]: doing byte:
myapp[5394]: eot - found?: true
myapp[5394]: done with line: <13>Mar 18 12:32:25 log-output: 200 OK
myapp[5394]: tag: 200

myapp[5394]: in parsetag cursor 32 - len 40
myapp[5394]: line: <13>Mar 18 12:32:25 log-output: Length:
myapp[5394]: in loop p.cursor: 32
myapp[5394]: doing byte: L
myapp[5394]: in loop p.cursor: 33
myapp[5394]: doing byte: e
myapp[5394]: in loop p.cursor: 34
myapp[5394]: doing byte: n
myapp[5394]: in loop p.cursor: 35
myapp[5394]: doing byte: g
myapp[5394]: in loop p.cursor: 36
myapp[5394]: doing byte: t
myapp[5394]: in loop p.cursor: 37
myapp[5394]: doing byte: h
myapp[5394]: in loop p.cursor: 38
myapp[5394]: doing byte: :
myapp[5394]: eot - found?: true
myapp[5394]: space
myapp[5394]: done with line: <13>Mar 18 12:32:25 log-output: Length:
myapp[5394]: tag: Length

myapp[5394]: in parsetag cursor 32 - len 34
myapp[5394]: line: <13>Mar 18 12:32:25 log-output: 15
myapp[5394]: in loop p.cursor: 32
myapp[5394]: doing byte: 1
myapp[5394]: in loop p.cursor: 33
myapp[5394]: doing byte: 5
myapp[5394]: in loop p.cursor: 34
myapp[5394]: panic: runtime error: index out of range
myapp[5394]: goroutine 35 [running]:
myapp[5394]: panic(0xa691c0, 0xc82000e020)

For reference:

func (p *Parser) parseTag() (string, error) {
    fmt.Printf("in parsetag cursor %d - len %d\n", p.cursor, p.l)
    fmt.Printf("line: %s\n", string(p.buff))

    var b byte
    var endOfTag bool
    var bracketOpen bool
    var tag []byte
    var err error
    var found bool

    from := p.cursor

    for {
        fmt.Printf("in loop p.cursor: %d\n", p.cursor)
        b = p.buff[p.cursor]
        fmt.Printf("doing byte: %s\n", string(b))
        bracketOpen = (b == '[')
        endOfTag = (b == ':' || b == ' ')

        // XXX : parse PID ?
        if bracketOpen {
            tag = p.buff[from:p.cursor]
            found = true
            fmt.Printf("bracket open\n")
        }

        if endOfTag {
            if !found {
                tag = p.buff[from:p.cursor]
                found = true
            }
            fmt.Printf("eot - found?: %+v\n", found)
            p.cursor++
            break
        }

        p.cursor++
    }

    if (p.cursor < p.l) && (p.buff[p.cursor] == ' ') {
        fmt.Printf("space\n")
        p.cursor++
    }

    fmt.Printf("done with line: %s\ntag: %s\n", p.buff, string(tag))

    return string(tag), err
}

from syslogparser.

Have opened MR #10 which seems to fix the issue in my testing.

from syslogparser.

@jeromer I just confirmed the same.

In particular:

• <13>Mar 18 11:32:10 log-output: panics
• <13>Mar 18 11:32:10 log-output: asdf panics
• <13>Mar 18 11:32:10 log-output: asdf doesn't panic but instead fails because it
  1. thinks "log-output:" is the hostname
  2. thinks "asdf" is the tag
  3. thinks " " is the content

from syslogparser.

Probably missing hostname, not tag

@Xiol Can you confirm that log-output was indeed a hostname and not a tag instead? The latter situation (i.e. a tag) seems more likely to have occurred from an RFC3164-compliant client.

1. Hostnames should only be ASCII lowercase letters, digits, or underscores, excluding :.
2. From section 4.1.2:

The TIMESTAMP will immediately follow the trailing ">" from the PRI
part and single space characters MUST follow each of the TIMESTAMP
and HOSTNAME fields.
...
...A single space character MUST also follow the HOSTNAME field.

3. From section 4.1.3:

Most commonly, the first character of the CONTENT field that signifies the
conclusion of the TAG field has been seen to be the left square
bracket character ("["), a colon character (":"), or a space
character.

If you know your client isn't setting a hostname, this can be mitigated by doing something like this:

p := NewParser(buff)
p.Hostname("whatever")
err := p.Parse()

See parseHostname() in rfc3164/rfc3164.go for why.

On the other hand, tags should only contain ASCII characters, unless they specify a PID, e.g. sshd[24521]. Seems like you have a funky client or relay in the mix there.

Parser is broken regardless

That said, the parser shouldn't be breaking on the above test cases. In particular, the parser should always be able to parse a record lacking a hostname, lacking a tag, lacking both, etc.

@jeromer I'll look into how this can be mitigated.

from syslogparser.