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

iana-time-zone v0.1.{43,44}: Use after free in MacOS / iOS implementation #1366

Merged
merged 1 commit into from
Aug 16, 2022
Merged

iana-time-zone v0.1.{43,44}: Use after free in MacOS / iOS implementation #1366

merged 1 commit into from
Aug 16, 2022

Conversation

Kijewski
Copy link
Contributor

In iana-time-zone v0.1.43 a use-after-free bug in the MacOS / iOS implementation was introduced.

The copied system time zone was released before its name was copied.
If the system time zone was changed between the call of CFRelease and str::to_owned(),
random memory would be copied.

Copy link
Contributor

@pinkforest pinkforest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome thank you! 💜

@pinkforest pinkforest added the Propose-Merge Propose-Merge label Aug 16, 2022
url = "https://github.com/strawlab/iana-time-zone/pull/54"
references = ["https://github.com/strawlab/iana-time-zone/pull/50#discussion_r945353515"]
categories = ["memory-corruption"]

Copy link
Contributor

@pinkforest pinkforest Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kijewski btw may want to add informational = "unsound" here so it goes more as a warning instead of full error

But up to you - ought to give an option which one to use :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review! Yes, informational = "unsound" sounds like the right categorization. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great - Thank you for being responsible and proactive maintainer - welcome aboard contributing 🚢 🚀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also just realised this may have been memory-exposure vs memory-corruption ?

corruption is where attacker can potentially modify memory where as exposure is exposing random bits

if you would like to fix the category feel free to send another PR :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I opened #1368. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ta. merged 🚢

In iana-time-zone v0.1.43 a use-after-free bug in the MacOS / iOS implementation was introduced.

The copied system time zone was released before its name was copied.
If the system time zone was changed between the call of `CFRelease()` and `str::to_owned()`,
random memory would be copied.
@pinkforest pinkforest merged commit afc10f8 into rustsec:main Aug 16, 2022
@pinkforest pinkforest added the memory exposure memory unintentionally exposed to attacker label Aug 16, 2022
@Kijewski Kijewski deleted the pr-iana-time-zone-v0.1.43 branch August 16, 2022 17:11
@pinkforest pinkforest removed the Propose-Merge Propose-Merge label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory exposure memory unintentionally exposed to attacker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants