From 21314e18383acb467f46185ec965fa0815335d13 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Fri, 2 Dec 2016 04:34:49 -0500 Subject: [PATCH] Fix TXT RDATA parsing (#421) * Test for proper parsing of whitespace-separated (TXT) character-strings * Properly parse whitespace-separated (TXT) character-strings * Remove non-RFC treatment of backslash sequences in character-strings Fixes gh-420 * For tests, remove non-RFC treatment of backslashes in domain names --- msg.go | 22 ---------- msg_helpers.go | 2 - parse_test.go | 19 +++++---- sanitize_test.go | 2 +- scan_rr.go | 105 +++++++++++++++++++++-------------------------- types.go | 19 +-------- update_test.go | 3 +- 7 files changed, 62 insertions(+), 110 deletions(-) diff --git a/msg.go b/msg.go index a9acd1e9..d99c2ddf 100644 --- a/msg.go +++ b/msg.go @@ -203,12 +203,6 @@ func packDomainName(s string, msg []byte, off int, compression map[string]int, c bs[j] = bs[j+2] } ls -= 2 - } else if bs[i] == 't' { - bs[i] = '\t' - } else if bs[i] == 'r' { - bs[i] = '\r' - } else if bs[i] == 'n' { - bs[i] = '\n' } escapedDot = bs[i] == '.' bsFresh = false @@ -335,10 +329,6 @@ Loop: fallthrough case '"', '\\': s = append(s, '\\', b) - case '\t': - s = append(s, '\\', 't') - case '\r': - s = append(s, '\\', 'r') default: if b < 32 || b >= 127 { // unprintable use \DDD var buf [3]byte @@ -431,12 +421,6 @@ func packTxtString(s string, msg []byte, offset int, tmp []byte) (int, error) { if i+2 < len(bs) && isDigit(bs[i]) && isDigit(bs[i+1]) && isDigit(bs[i+2]) { msg[offset] = dddToByte(bs[i:]) i += 2 - } else if bs[i] == 't' { - msg[offset] = '\t' - } else if bs[i] == 'r' { - msg[offset] = '\r' - } else if bs[i] == 'n' { - msg[offset] = '\n' } else { msg[offset] = bs[i] } @@ -508,12 +492,6 @@ func unpackTxtString(msg []byte, offset int) (string, int, error) { switch b { case '"', '\\': s = append(s, '\\', b) - case '\t': - s = append(s, `\t`...) - case '\r': - s = append(s, `\r`...) - case '\n': - s = append(s, `\n`...) default: if b < 32 || b > 127 { // unprintable var buf [3]byte diff --git a/msg_helpers.go b/msg_helpers.go index e7a9500c..494c0537 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -263,8 +263,6 @@ func unpackString(msg []byte, off int) (string, int, error) { switch b { case '"', '\\': s = append(s, '\\', b) - case '\t', '\r', '\n': - s = append(s, b) default: if b < 32 || b > 127 { // unprintable var buf [3]byte diff --git a/parse_test.go b/parse_test.go index ca467a22..dc18b59c 100644 --- a/parse_test.go +++ b/parse_test.go @@ -86,7 +86,7 @@ func TestDomainName(t *testing.T) { } func TestDomainNameAndTXTEscapes(t *testing.T) { - tests := []byte{'.', '(', ')', ';', ' ', '@', '"', '\\', '\t', '\r', '\n', 0, 255} + tests := []byte{'.', '(', ')', ';', ' ', '@', '"', '\\', 9, 13, 10, 0, 255} for _, b := range tests { rrbytes := []byte{ 1, b, 0, // owner @@ -127,8 +127,8 @@ func TestTXTEscapeParsing(t *testing.T) { test := [][]string{ {`";"`, `";"`}, {`\;`, `";"`}, - {`"\t"`, `"\t"`}, - {`"\r"`, `"\r"`}, + {`"\t"`, `"t"`}, + {`"\r"`, `"r"`}, {`"\ "`, `" "`}, {`"\;"`, `";"`}, {`"\;\""`, `";\""`}, @@ -137,8 +137,9 @@ func TestTXTEscapeParsing(t *testing.T) { {`"(a\)"`, `"(a)"`}, {`"(a)"`, `"(a)"`}, {`"\048"`, `"0"`}, - {`"\` + "\n" + `"`, `"\n"`}, - {`"\` + "\r" + `"`, `"\r"`}, + {`"\` + "\t" + `"`, `"\009"`}, + {`"\` + "\n" + `"`, `"\010"`}, + {`"\` + "\r" + `"`, `"\013"`}, {`"\` + "\x11" + `"`, `"\017"`}, {`"\'"`, `"'"`}, } @@ -417,16 +418,16 @@ func TestQuotes(t *testing.T) { tests := map[string]string{ `t.example.com. IN TXT "a bc"`: "t.example.com.\t3600\tIN\tTXT\t\"a bc\"", `t.example.com. IN TXT "a - bc"`: "t.example.com.\t3600\tIN\tTXT\t\"a\\n bc\"", + bc"`: "t.example.com.\t3600\tIN\tTXT\t\"a\\010 bc\"", `t.example.com. IN TXT ""`: "t.example.com.\t3600\tIN\tTXT\t\"\"", `t.example.com. IN TXT "a"`: "t.example.com.\t3600\tIN\tTXT\t\"a\"", `t.example.com. IN TXT "aa"`: "t.example.com.\t3600\tIN\tTXT\t\"aa\"", `t.example.com. IN TXT "aaa" ;`: "t.example.com.\t3600\tIN\tTXT\t\"aaa\"", `t.example.com. IN TXT "abc" "DEF"`: "t.example.com.\t3600\tIN\tTXT\t\"abc\" \"DEF\"", `t.example.com. IN TXT "abc" ( "DEF" )`: "t.example.com.\t3600\tIN\tTXT\t\"abc\" \"DEF\"", - `t.example.com. IN TXT aaa ;`: "t.example.com.\t3600\tIN\tTXT\t\"aaa \"", - `t.example.com. IN TXT aaa aaa;`: "t.example.com.\t3600\tIN\tTXT\t\"aaa aaa\"", - `t.example.com. IN TXT aaa aaa`: "t.example.com.\t3600\tIN\tTXT\t\"aaa aaa\"", + `t.example.com. IN TXT aaa ;`: "t.example.com.\t3600\tIN\tTXT\t\"aaa\"", + `t.example.com. IN TXT aaa aaa;`: "t.example.com.\t3600\tIN\tTXT\t\"aaa\" \"aaa\"", + `t.example.com. IN TXT aaa aaa`: "t.example.com.\t3600\tIN\tTXT\t\"aaa\" \"aaa\"", `t.example.com. IN TXT aaa`: "t.example.com.\t3600\tIN\tTXT\t\"aaa\"", "cid.urn.arpa. NAPTR 100 50 \"s\" \"z3950+I2L+I2C\" \"\" _z3950._tcp.gatech.edu.": "cid.urn.arpa.\t3600\tIN\tNAPTR\t100 50 \"s\" \"z3950+I2L+I2C\" \"\" _z3950._tcp.gatech.edu.", "cid.urn.arpa. NAPTR 100 50 \"s\" \"rcds+I2C\" \"\" _rcds._udp.gatech.edu.": "cid.urn.arpa.\t3600\tIN\tNAPTR\t100 50 \"s\" \"rcds+I2C\" \"\" _rcds._udp.gatech.edu.", diff --git a/sanitize_test.go b/sanitize_test.go index c108dc69..2ba3fe9a 100644 --- a/sanitize_test.go +++ b/sanitize_test.go @@ -65,7 +65,7 @@ func TestNormalizedString(t *testing.T) { tests := map[RR]string{ newRR(t, "mIEk.Nl. 3600 IN A 127.0.0.1"): "miek.nl.\tIN\tA\t127.0.0.1", newRR(t, "m\\ iek.nL. 3600 IN A 127.0.0.1"): "m\\ iek.nl.\tIN\tA\t127.0.0.1", - newRR(t, "m\\\tIeK.nl. 3600 in A 127.0.0.1"): "m\\tiek.nl.\tIN\tA\t127.0.0.1", + newRR(t, "m\\\tIeK.nl. 3600 in A 127.0.0.1"): "m\\009iek.nl.\tIN\tA\t127.0.0.1", } for tc, expected := range tests { n := normalizedString(tc) diff --git a/scan_rr.go b/scan_rr.go index 675fc80d..00098ad7 100644 --- a/scan_rr.go +++ b/scan_rr.go @@ -64,74 +64,63 @@ func endingToString(c chan lex, errstr, f string) (string, *ParseError, string) return s, nil, l.comment } -// A remainder of the rdata with embedded spaces, return the parsed string slice (sans the spaces) -// or an error +// A remainder of the rdata with embedded spaces, split on unquoted whitespace +// and return the parsed string slice or an error func endingToTxtSlice(c chan lex, errstr, f string) ([]string, *ParseError, string) { // Get the remaining data until we see a zNewline - quote := false l := <-c - var s []string if l.err { - return s, &ParseError{f, errstr, l}, "" + return nil, &ParseError{f, errstr, l}, "" } - switch l.value == zQuote { - case true: // A number of quoted string - s = make([]string, 0) - empty := true - for l.value != zNewline && l.value != zEOF { - if l.err { - return nil, &ParseError{f, errstr, l}, "" - } - switch l.value { - case zString: - empty = false - if len(l.token) > 255 { - // split up tokens that are larger than 255 into 255-chunks - sx := []string{} - p, i := 0, 255 - for { - if i <= len(l.token) { - sx = append(sx, l.token[p:i]) - } else { - sx = append(sx, l.token[p:]) - break - } - p, i = p+255, i+255 - } - s = append(s, sx...) - break - } - - s = append(s, l.token) - case zBlank: - if quote { - // zBlank can only be seen in between txt parts. - return nil, &ParseError{f, errstr, l}, "" - } - case zQuote: - if empty && quote { - s = append(s, "") - } - quote = !quote - empty = true - default: - return nil, &ParseError{f, errstr, l}, "" - } - l = <-c - } - if quote { + // Build the slice + s := make([]string, 0) + quote := false + empty := false + for l.value != zNewline && l.value != zEOF { + if l.err { return nil, &ParseError{f, errstr, l}, "" } - case false: // Unquoted text record - s = make([]string, 1) - for l.value != zNewline && l.value != zEOF { - if l.err { - return s, &ParseError{f, errstr, l}, "" + switch l.value { + case zString: + empty = false + if len(l.token) > 255 { + // split up tokens that are larger than 255 into 255-chunks + sx := []string{} + p, i := 0, 255 + for { + if i <= len(l.token) { + sx = append(sx, l.token[p:i]) + } else { + sx = append(sx, l.token[p:]) + break + + } + p, i = p+255, i+255 + } + s = append(s, sx...) + break } - s[0] += l.token - l = <-c + + s = append(s, l.token) + case zBlank: + if quote { + // zBlank can only be seen in between txt parts. + return nil, &ParseError{f, errstr, l}, "" + } + case zQuote: + if empty && quote { + s = append(s, "") + } + quote = !quote + empty = true + default: + return nil, &ParseError{f, errstr, l}, "" } + l = <-c + } + if quote { + return nil, &ParseError{f, errstr, l}, "" } return s, nil, l.comment } diff --git a/types.go b/types.go index f63a18b3..c8b3191e 100644 --- a/types.go +++ b/types.go @@ -480,12 +480,6 @@ func appendDomainNameByte(s []byte, b byte) []byte { func appendTXTStringByte(s []byte, b byte) []byte { switch b { - case '\t': - return append(s, '\\', 't') - case '\r': - return append(s, '\\', 'r') - case '\n': - return append(s, '\\', 'n') case '"', '\\': return append(s, '\\', b) } @@ -525,17 +519,8 @@ func nextByte(b []byte, offset int) (byte, int) { return dddToByte(b[offset+1:]), 4 } } - // not \ddd, maybe a control char - switch b[offset+1] { - case 't': - return '\t', 2 - case 'r': - return '\r', 2 - case 'n': - return '\n', 2 - default: - return b[offset+1], 2 - } + // not \ddd, just an RFC 1035 "quoted" character + return b[offset+1], 2 } type SPF struct { diff --git a/update_test.go b/update_test.go index 56602dfe..8ecd4131 100644 --- a/update_test.go +++ b/update_test.go @@ -10,7 +10,8 @@ func TestDynamicUpdateParsing(t *testing.T) { for _, typ := range TypeToString { if typ == "OPT" || typ == "AXFR" || typ == "IXFR" || typ == "ANY" || typ == "TKEY" || typ == "TSIG" || typ == "ISDN" || typ == "UNSPEC" || typ == "NULL" || typ == "ATMA" || - typ == "Reserved" || typ == "None" || typ == "NXT" || typ == "MAILB" || typ == "MAILA" { + typ == "Reserved" || typ == "None" || typ == "NXT" || typ == "MAILB" || typ == "MAILA" || + typ == "UINFO" { continue } r, err := NewRR(prefix + typ)