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

Update documentation for format strings #2501

Merged
merged 3 commits into from
Dec 8, 2024

Conversation

Legoclones
Copy link
Contributor

By default, when creating a FmtStr class or using fmtstr_payload(), all the documentation (and examples) state that the value in the writes dictionary should be an int. However, this int is automatically expanded to a bytestring of size long internally. If someone wants to write data shorter than a long, they need to provide a bytestring as the value instead of an integer (either raw using b'\x37\x13' or helper functions like p16(0x1337)). However, this is never documented anywhere or provided in any examples.

This has led to a number of issues being filed, such as:

This pull requests expands on the current documentation to explicitly lay out whether an int or bytes argument is needed and examples are provided to show how providing a bytestring creates a different format string payload.

Hopefully this helps to reduce confusion and prevent similar future issues from being filed.

Clarified that the `value` in the `writes` dict (for `fmtstr_payload()`) should be a bytestring if the value to be written is shorter than a long
Updated example and definition for `write()` function in `FmtStr` class to include bytes as the value.
Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

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

Thank you! ❤️

@peace-maker peace-maker merged commit 55ac6e1 into Gallopsled:dev Dec 8, 2024
6 of 10 checks passed
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 this pull request may close these issues.

2 participants