Skip to content

Fix overflow when searching for #> #7094

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

Merged
merged 1 commit into from
Jan 27, 2017
Merged

Fix overflow when searching for #> #7094

merged 1 commit into from
Jan 27, 2017

Conversation

bitjammer
Copy link
Contributor

ede6bf7 increments the buffer pointer to early
when searching for the <# prefix to stop lexing an operator,
so the source buffer is accessed 1 byte off the end.

Thanks ASan and thanks @gparker42!

rdar://problem/28457876

@bitjammer
Copy link
Contributor Author

Doing an ASan run locally.

@bitjammer
Copy link
Contributor Author

I knew there was a reason my body woke me up early today ...

@bitjammer bitjammer changed the title Fix overflow when searching for <# Fix overflow when searching for #> Jan 27, 2017
@bitjammer
Copy link
Contributor Author

@swift-ci Please smoke test

@gparker42
Copy link
Contributor

rangeContainsPlaceholderEnd() looks good.

Is the call to isEditorPlaceholder() correct? I think it blows up if the buffer ends with <.

@bitjammer
Copy link
Contributor Author

Oh, hm. So that might also go past the buffer by one?

@bitjammer
Copy link
Contributor Author

Since isEditorPlaceHolder is just a startswith call, we could do this instead there, yeah?

Identifier::isEditorPlaceholder(StringRef(CurPtr, BufferEnd-CurPtr))

ede6bf7 increments the buffer pointer to early
when searching for the <# prefix to stop lexing an operator,
so the source buffer is accessed 1 byte off the end.

Thanks ASan and thanks @gparker42!

rdar://problem/28457876
@bitjammer
Copy link
Contributor Author

@swift-ci Please smoke test

@bitjammer
Copy link
Contributor Author

@swift-ci Please smoke test f'real

@bitjammer
Copy link
Contributor Author

@swift-ci Pretty please smoke test

@gparker42
Copy link
Contributor

BufferEnd-CurPtr looks safe. We already know that CurPtr is not past BufferEnd.

@bitjammer
Copy link
Contributor Author

@swift-ci Please asan test

@bitjammer
Copy link
Contributor Author

The ASan test is running but not showing up here: https://ci.swift.org/job/swift-PR-osx-ASAN-test/46

@gparker42
Copy link
Contributor

ASAN test succeeded. This should be good to go.

@bitjammer bitjammer merged commit f731d40 into swiftlang:master Jan 27, 2017
@bitjammer bitjammer deleted the rdar-28457876-fix-asan-buffer-overflow branch January 27, 2017 22:21
@bitjammer
Copy link
Contributor Author

Thanks again @gparker42 !

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