Skip to content

Fixes for: heap-use-after-free in parser error handling; out-of-range in special_number #2755

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Nov 23, 2018

Conversation

glebm
Copy link
Contributor

@glebm glebm commented Nov 22, 2018

Fixes #2643: pstate.src may not outlive stack unwind so we must copy it.

Also optimizes line_begin/end search in handle_error. There is no need to advance by UTF-8 code points when searching for an ASCII character, because UTF-8 is a prefix-free encoding.

@xzyfer
Copy link
Contributor

xzyfer commented Nov 22, 2018 via email

There is no need to advance by UTF-8 code points when searching for an
ASCII character, because UTF-8 is a prefix-free encoding.
@glebm
Copy link
Contributor Author

glebm commented Nov 22, 2018

Fixed! This was the cause of both Travis and AppVeyor failing: 436ee57

green-galaxy

Well the plugin tests are still failing ASan but only due to bugs in the plugins (for which the PRs have been sent already).

Once this has been merged I'll rebase #2754.

@glebm glebm changed the title Fix heap-use-after-free in Parser error handling Fixes for: heap-use-after-free in parser error handling; out-of-range in special_number Nov 22, 2018
Out-of-range string access happened when `s->value()` was shorter than "var(" or "calc(".
@glebm
Copy link
Contributor Author

glebm commented Nov 23, 2018

Ah, nevermind, Windows still failing. The whack-a-mole continues.

@glebm
Copy link
Contributor Author

glebm commented Nov 23, 2018

Not reproducible on Visual Studio 2015 and Visual Studio 2017, so I'm downloading Visual Studio 2012 now.

@glebm
Copy link
Contributor Author

glebm commented Nov 23, 2018

Oh, Microsoft Visual Studio 12.0 is actually Visual Studio 2013. 😐

@glebm
Copy link
Contributor Author

glebm commented Nov 23, 2018

Finally fixed. Learned that exceptions must have a copy constructor (https://stackoverflow.com/a/10855545). In my day job we don't use exceptions.

The copy constructor transfers the ownership of `owned_src` from `rhs` to `lhs`.
Otherwise, `owned_src` may be destroyed too early and more than once.

In the libsass codebase, this copy can only happen on `throw`.

Modern compilers elide such copies. In our CI, only VS 2013 does not.
@xzyfer
Copy link
Contributor

xzyfer commented Nov 23, 2018

Whoa awesome work!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AddressSanitizer: heap-use-after-free in libsass
2 participants