Fix buffer overflow / memory leak in mapuvraster.c #5148

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
@gogglesguy
Contributor

gogglesguy commented Aug 21, 2015

Using uvlinfo->height instead of uvlinfo->width will either get you a buffer overflow or memory leak depending on whether width or height is larger.

gogglesguy added some commits Aug 21, 2015

Fix incorrect comparison to -1 for itemindexes initialization
itemindexes gets freshly allocated and never gets any -1 assignment. Use else instead.
Fix incorrect refcount
In the first LayerOpen call, data gets alloacted and refcount gets initialized to 1. Calling LayerClose should do the opposite, however comparing refcount < 0 requires an additional LayerClose. Use refcount < 1 instead.
@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Aug 21, 2015

Member

@dmorissette, something you can look at?

Member

sdlime commented Aug 21, 2015

@dmorissette, something you can look at?

@gogglesguy

This comment has been minimized.

Show comment
Hide comment
@gogglesguy

gogglesguy Aug 21, 2015

Contributor

So checking for -1 for itemindexes on a uninitialized array was wrong. Returning from the item loop early in msUVRASTERLayerInitItemInfo would result in uninitialized values in itemindexes and eventually propagated to values as well. Perhaps because msUVRASTERLayerGetItems was never called?

Contributor

gogglesguy commented Aug 21, 2015

So checking for -1 for itemindexes on a uninitialized array was wrong. Returning from the item loop early in msUVRASTERLayerInitItemInfo would result in uninitialized values in itemindexes and eventually propagated to values as well. Perhaps because msUVRASTERLayerGetItems was never called?

@gogglesguy

This comment has been minimized.

Show comment
Hide comment
@gogglesguy

gogglesguy Aug 24, 2015

Contributor

Although the testsuit fails (it no longer crashes, yay!), i believe the output images look fine. I'm sure @dmorissette can take a look at this :)

Contributor

gogglesguy commented Aug 24, 2015

Although the testsuit fails (it no longer crashes, yay!), i believe the output images look fine. I'm sure @dmorissette can take a look at this :)

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Sep 3, 2015

Contributor

I hate to be blocking this but don't think I'll be able to look into this anytime soon. As I'm not the author of the code anyway, any other committer would be as good as me to reproduce/test and validate the fix

Contributor

dmorissette commented Sep 3, 2015

I hate to be blocking this but don't think I'll be able to look into this anytime soon. As I'm not the author of the code anyway, any other committer would be as good as me to reproduce/test and validate the fix

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Sep 8, 2015

Member

applied/squashed in d6d3432, thanks

Member

tbonfort commented Sep 8, 2015

applied/squashed in d6d3432, thanks

@tbonfort tbonfort closed this Sep 8, 2015

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Sep 8, 2015

Contributor

Thank you @tbonfort

Contributor

dmorissette commented Sep 8, 2015

Thank you @tbonfort

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment