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

elf: prevent overflow #243

Merged
merged 1 commit into from
Nov 17, 2020
Merged

elf: prevent overflow #243

merged 1 commit into from
Nov 17, 2020

Conversation

jackcmay
Copy link
Contributor

Fuzzier found an overflow if bad section address or size.

  • Update check_size to also check addr + size
  • Use saturating addition to prevent overflow or a wrap-around region bound if check_size is not called on a created section.

@jackcmay
Copy link
Contributor Author

Looks like CI is failing on something unrelated to my changes :-(

@m4b
Copy link
Owner

m4b commented Nov 17, 2020

Yes looks like it's this: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

aside: i find it a bit sad that for nearly 4 years travis CI did not have a single CI involved regression with any of the github projects i had, and now, with not even 1 year into github actions, it's already breaking my code, and wasting developer time upgrading to some new api... This does not bode well.

@jackcmay
Copy link
Contributor Author

:-(. Should I rebase once you have applied a fix?

@m4b
Copy link
Owner

m4b commented Nov 17, 2020

I don't have the time right now to fix the issue; maybe this weekend; i can just merge without CI if you want

Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

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

LGTM thanks for this!

@m4b
Copy link
Owner

m4b commented Nov 17, 2020

If you think you know how to fix, feel free to go for it in this PR, otherwise we can just merge

@m4b
Copy link
Owner

m4b commented Nov 17, 2020

Ok i think i fixed it actually here: #244

can you rebase on top of that ?

@jackcmay
Copy link
Contributor Author

Rebased and CI passed, woot!

@m4b
Copy link
Owner

m4b commented Nov 17, 2020

sorrrryyyyy for spamming :D and thanks for the PR ❤️

@m4b m4b merged commit 6495cfe into m4b:master Nov 17, 2020
@jackcmay jackcmay deleted the prevent-overflow branch November 17, 2020 06:41
@jackcmay
Copy link
Contributor Author

@m4b When do you think these changes will be released so we can pick them up?

@m4b
Copy link
Owner

m4b commented Nov 26, 2020

whoops, forgot about this, will release now!

@m4b
Copy link
Owner

m4b commented Nov 26, 2020

ok this is released in 0.3, which rolls up several things from past few months; it's a breaking change because one part of the mach-o api changed, but otherwise it should be fine

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