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 Issue 18190 - [asan] heap-buffer-overflow in Module.load.checkMod… #7593

Merged
merged 1 commit into from Jan 4, 2018

Conversation

WalterBright
Copy link
Member

…FileAlias

I believe the problem is that memcmp is allowed to read nbytes of data, even past the byte where a mismatch occurs. The code assumed it did not read past the mismatch.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Description
18190 [asan] heap-buffer-overflow in Module.load.checkModFileAlias

@dlang-bot dlang-bot merged commit 7ddec7d into dlang:master Jan 4, 2018
@JohanEngelen
Copy link
Contributor

I believe the problem is that memcmp is allowed to read nbytes of data, even past the byte where a mismatch occurs. The code assumed it did not read past the mismatch.

Indeed it appears so: https://trust-in-soft.com/memcmp-requires-pointers-to-fully-valid-buffers/
This is also a problem then in the cmdline args matching code in LDC's LDMD and in mars.d.

@WalterBright
Copy link
Member Author

@JohanEngelen thanks for doing the research to confirm. I had assumed forever that it wouldn't read past the mismatch, but when I read the memcmp description last night, it didn't actually say that. We should always assume the worst when things are not explicitly stated.

This also means searching the source code for memcmp and double checking them.

@JohanEngelen
Copy link
Contributor

And @kinke has found the trivial replacement for the string checking memcmp: strncmp.
https://stackoverflow.com/questions/38878195/does-this-usage-of-strncmp-contain-an-out-of-bounds-read

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants