Skip to content

Comments

r.plane: remove misused gisprompt option#5601

Merged
nilason merged 2 commits intoOSGeo:mainfrom
nilason:fix_r_plane_heap_overflox
Apr 29, 2025
Merged

r.plane: remove misused gisprompt option#5601
nilason merged 2 commits intoOSGeo:mainfrom
nilason:fix_r_plane_heap_overflox

Conversation

@nilason
Copy link
Contributor

@nilason nilason commented Apr 27, 2025

As the option does not contain a comma separated three part string, this causes heap buffer overflow. This is probably the cause of some random CI failures with r.plane.

Follow-up to #5493.

As the option does not contain a comma separated three part string,
this causes heap buffer overflow.
@nilason nilason added this to the 8.5.0 milestone Apr 27, 2025
@nilason nilason added backport to 8.4 PR needs to be backported to release branch 8.4 bug Something isn't working labels Apr 27, 2025
@github-actions github-actions bot added raster Related to raster data processing Python Related code is in Python module labels Apr 27, 2025
@wenzeslaus
Copy link
Member

It looks like "options" or whatever the allowed values are called.

@nilason
Copy link
Contributor Author

nilason commented Apr 28, 2025

It looks like "options" or whatever the allowed values are called.

They are passed to the module parser and G__split_gisprompt() assumes the option value is correct:

void G__split_gisprompt(const char *gisprompt, char *age, char *element,

During compilation this is triggered by documentation generation (calling parser html|md).

@wenzeslaus
Copy link
Member

@nilason
Copy link
Contributor Author

nilason commented Apr 28, 2025

I don't doubt this fails. What I meant is that there is options and the current values look like the value to set there.

example: https://github.com/OSGeo/grass/blob/main/raster/r.viewshed/main.cpp#L509 doc: https://grass.osgeo.org/programming8/structOption.html#aaf64185e77299e332f9fcb4880a0507b

Aha, now I see what you mean, you suggest something like:

- # % gisprompt: -90-90
+ # % options: -90-90

@wenzeslaus
Copy link
Member

Yes!

@nilason
Copy link
Contributor Author

nilason commented Apr 28, 2025

Now I changed to use "options".

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Thanks!

@nilason nilason merged commit 14f5310 into OSGeo:main Apr 29, 2025
28 checks passed
neteler pushed a commit that referenced this pull request Apr 29, 2025
Replace misused ’gisprompt’ option to ’options’.

As the gisprompt option does not contain a comma separated three part string,
this causes heap buffer overflow.
@neteler neteler removed the backport to 8.4 PR needs to be backported to release branch 8.4 label Apr 29, 2025
@neteler neteler modified the milestones: 8.5.0, 8.4.2 Apr 29, 2025
@nilason nilason deleted the fix_r_plane_heap_overflox branch May 9, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working module Python Related code is in Python raster Related to raster data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants