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

Display for NSString truncates at null bytes #14

Closed
dylni opened this issue Oct 31, 2021 · 6 comments · Fixed by rustsec/advisory-db#1223
Closed

Display for NSString truncates at null bytes #14

dylni opened this issue Oct 31, 2021 · 6 comments · Fixed by rustsec/advisory-db#1223

Comments

@dylni
Copy link

dylni commented Oct 31, 2021

This issue was originally reported privately, but I thought I should create an issue to inform others about it, as it has not yet been fixed.

The implementation of Display for NSString truncates at null bytes, since it uses CStr::from_ptr. It should be possible to use [NSString lengthOfBytesUsingEncoding:] and [NSString UTF8String] instead to create the complete string.

Example:

let string = NSString::from_str("null\0bytes");
println!("{}", string);

That example only prints the string "null". Since ToString::to_string uses Display to create strings, it has the same issue. NSString::to_str and NSString::to_str_with_null also have the same issue.

If you believe this is a valid issue, I encourage you to file a security advisory at https://github.com/rustsec/advisory-db, including a patch version if it is possible to release within two weeks.

Thank you for your work on this crate. Creating a security advisory is not meant to be anything other than a way to provide information to other users.

Shnatsel pushed a commit to rustsec/advisory-db that referenced this issue Nov 15, 2021
* Add fruity advisory for nvzqz/fruity#14

* Fix lint error

* Fix lint error

* Add an impact section
@Shnatsel
Copy link

Heads up: this issue has been included in the RustSec advisory database. It will be surfaced by tools such as cargo-audit or cargo-deny from now on.

Once a fix is released to crates.io, please open a pull request to update the advisory with the patched version, or file an issue on the advisory database repository.

@nvzqz
Copy link
Owner

nvzqz commented Apr 8, 2022

This is now fixed in release 0.3.0. Sorry for the delay. I cherry-picked it from a series of changes that I wanted to clean up and didn't get around to. I will be adding more APIs and releases soon, with a focus on macOS.

@nvzqz nvzqz closed this as completed Apr 8, 2022
@dylni
Copy link
Author

dylni commented Apr 18, 2022

Thanks @nvzqz! The new implementation looks good and appears to resolve the issue. Also, NSString::from_str_no_copy should no longer need a "Safety" section, since it's safe now.

@nvzqz
Copy link
Owner

nvzqz commented Apr 18, 2022

@dylni that method is safe because the NSString cannot outlive the input due to the 'data lifetime. I forgot to remove the safety notice.

@dylni
Copy link
Author

dylni commented Apr 18, 2022

@nvzqz Sorry, I realized my original comment was wrong and edited it. You are correct that the lifetime is now sufficiently enforced.

@nvzqz
Copy link
Owner

nvzqz commented Apr 18, 2022

No worries. I appreciate your diligence. Things like that do get missed.

madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 21, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 22, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 22, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 22, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 22, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 26, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
madsmtm added a commit to madsmtm/objc2 that referenced this issue Jul 26, 2022
If the user needs the string to end with e.g. two NUL, we really shouldn't stop them.

`fruity` probably did this originally because their NSString truncated at null bytes, see nvzqz/fruity#14
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 a pull request may close this issue.

3 participants