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

Prevent potential buffer overflow in sscanf #663

Merged
merged 1 commit into from Jul 23, 2014
Merged

Prevent potential buffer overflow in sscanf #663

merged 1 commit into from Jul 23, 2014

Conversation

moshekaplan
Copy link
Contributor

Unbounded reads with sscanf can cause a buffer overflow when given an overly long string.

In this case, an adversary capable of modifying the .ini file would be able to trigger a buffer overflow in dolphin.

@delroth
Copy link
Member

delroth commented Jul 23, 2014

TBH an adversary with local access to the filesystem can do way worse things than exploit Dolphin... it provides no escalation path and thus is not really a security issue.

No reason to keep these kind of bugs though.

@dolphin-emu-bot rebuild

@Sonicadvance1
Copy link
Contributor

Woo Android security!

@lioncash
Copy link
Member

For what it's worth, we could probably use direct string handling + std::stoi and the family of similar funcs (or stringstream) and avoid the use of sscanf entirely.

Either way though, this looks fine.

@Sonicadvance1
Copy link
Contributor

Yea. the string handling really should be improved.
Other priorities right now of course.

Sonicadvance1 added a commit that referenced this pull request Jul 23, 2014
Prevent potential buffer overflow in sscanf
@Sonicadvance1 Sonicadvance1 merged commit e91db62 into dolphin-emu:master Jul 23, 2014
@moshekaplan moshekaplan deleted the patch-1 branch July 23, 2014 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants