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 possible out-of-bounds read in endingToTxtSlice #1557

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 24 additions & 14 deletions scan_rr.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ func endingToTxtSlice(c *zlexer, errstr string) ([]string, *ParseError) {
sx := []string{}
p := 0
for {
i := escapedStringOffset(l.token[p:], 255)
i, ok := escapedStringOffset(l.token[p:], 255)
if !ok {
return nil, &ParseError{err: errstr, lex: l}
}
if i != -1 && p+i != len(l.token) {
sx = append(sx, l.token[p:p+i])
} else {
Expand Down Expand Up @@ -1919,29 +1922,36 @@ func (rr *APL) parse(c *zlexer, o string) *ParseError {

// escapedStringOffset finds the offset within a string (which may contain escape
// sequences) that corresponds to a certain byte offset. If the input offset is
// out of bounds, -1 is returned.
func escapedStringOffset(s string, byteOffset int) int {
if byteOffset == 0 {
return 0
// out of bounds, -1 is returned (which is *not* considered an error).
func escapedStringOffset(s string, desiredByteOffset int) (int, bool) {
if desiredByteOffset == 0 {
return 0, true
}

offset := 0
for i := 0; i < len(s); i++ {
offset += 1
currentByteOffset, i := 0, 0

for i < len(s) {
currentByteOffset += 1

// Skip escape sequences
if s[i] != '\\' {
// Not an escape sequence; nothing to do.
// Single plain byte, not an escape sequence.
i++
} else if isDDD(s[i+1:]) {
i += 3
// Skip backslash and DDD.
i += 4
} else if len(s[i+1:]) < 1 {
// No character following the backslash; that's an error.
return 0, false
} else {
i++
// Skip backslash and following byte.
i += 2
}

if offset >= byteOffset {
return i + 1
if currentByteOffset >= desiredByteOffset {
return i, true
}
}

return -1
return -1, true
}
34 changes: 23 additions & 11 deletions scan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,25 +433,37 @@ func TestEscapedStringOffset(t *testing.T) {
input string
inputOffset int
expectedOffset int
expectedOK bool
}{
{"simple string with no escape sequences", 20, 20},
{"simple string with no escape sequences", 500, -1},
{`\;\088\\\;\120\\`, 0, 0},
{`\;\088\\\;\120\\`, 1, 2},
{`\;\088\\\;\120\\`, 2, 6},
{`\;\088\\\;\120\\`, 3, 8},
{`\;\088\\\;\120\\`, 4, 10},
{`\;\088\\\;\120\\`, 5, 14},
{`\;\088\\\;\120\\`, 6, 16},
{`\;\088\\\;\120\\`, 7, -1},
{"simple string with no escape sequences", 20, 20, true},
{"simple string with no escape sequences", 500, -1, true},
{`\;\088\\\;\120\\`, 0, 0, true},
{`\;\088\\\;\120\\`, 1, 2, true},
{`\;\088\\\;\120\\`, 2, 6, true},
{`\;\088\\\;\120\\`, 3, 8, true},
{`\;\088\\\;\120\\`, 4, 10, true},
{`\;\088\\\;\120\\`, 5, 14, true},
{`\;\088\\\;\120\\`, 6, 16, true},
{`\;\088\\\;\120\\`, 7, -1, true},
{`\`, 3, 0, false},
{`a\`, 3, 0, false},
{`aa\`, 3, 0, false},
{`aaa\`, 3, 3, true},
{`aaaa\`, 3, 3, true},
}
for i, test := range cases {
outputOffset := escapedStringOffset(test.input, test.inputOffset)
outputOffset, outputOK := escapedStringOffset(test.input, test.inputOffset)
if outputOffset != test.expectedOffset {
t.Errorf(
"Test %d (input %#q offset %d) returned offset %d but expected %d",
i, test.input, test.inputOffset, outputOffset, test.expectedOffset,
)
}
if outputOK != test.expectedOK {
t.Errorf(
"Test %d (input %#q offset %d) returned ok=%t but expected %t",
i, test.input, test.inputOffset, outputOK, test.expectedOK,
)
}
}
}