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

astyle: Replace sorting implementation #58736

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

tygyh
Copy link
Contributor

@tygyh tygyh commented Sep 13, 2024

The following vulnerabilities are fixed with an upgrade:

Description

Replace the sorting implementation since the previous implementation might cause a buffer overread.

@github-actions github-actions bot added this to the 3.40.0 milestone Sep 13, 2024
@rouault
Copy link
Contributor

rouault commented Sep 13, 2024

since the previous implementation might cause a buffer overread.

I don't think so. Can you prove it ?
Note that the C standard allows "one-past-the-end pointer", so &fileName[fileName.size()] is legal.
Cf https://stackoverflow.com/questions/16233868/pointer-comparisons-with-one-past-the-last-element-of-an-array-object and https://port70.net/~nsz/c/c11/n1570.html#6.5.6p8

But your proposal looks slightly more elegant

@rouault
Copy link
Contributor

rouault commented Sep 13, 2024

Note that the C standard allows "one-past-the-end pointer", so &fileName[fileName.size()] is legal.

Apologies. On reflection, I'm likely wrong. fileName.data() + fileName.size() would be legal, but here we formally dereference an out-of-range index, before taking its address. So a pedantic analyzer would likely complain about that

@rouault rouault changed the title Replace sorting implementation astyle: Replace sorting implementation Sep 13, 2024
Copy link

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 52e3d5b)

@nyalldawson nyalldawson merged commit 4e8e5da into qgis:master Sep 16, 2024
32 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.

3 participants