Skip to content
This repository has been archived by the owner on Apr 17, 2022. It is now read-only.

Segfault in map.h line 208 caused by invalid map size 0x0 #2135

Closed
wzdev-ci opened this issue Sep 3, 2010 · 11 comments
Closed

Segfault in map.h line 208 caused by invalid map size 0x0 #2135

wzdev-ci opened this issue Sep 3, 2010 · 11 comments

Comments

@wzdev-ci
Copy link
Contributor

wzdev-ci commented Sep 3, 2010

keyword_segfault_map_size_invalid resolution_fixed type_bug | by corvuscorax@...


map.h line 208:

...
206 static inline unsigned char terrainType(const MAPTILE * tile)
207 {
208    return terrainTypes[TileNumber_tile(tile->texture)];
209 }
...

According to backtrace it was called from

map_Height(), map.c, line 1190

...
if (terrainType(mapTile(tileX,tileY)) == TER_WATER)
...

which in turn was called from
moveUpdateDroid(), move.c, line 3000,

the invalid tile pointer came from:

static inline WZ_DECL_PURE MAPTILE *mapTile(SDWORD x, SDWORD y)
/* Return a pointer to the tile structure at x,y */
static inline WZ_DECL_PURE MAPTILE *mapTile(SDWORD x, SDWORD y)
{
	// Clamp x and y values to actual ones
	// Give one tile worth of leeway before asserting, for units/transporters coming in from off-map.
	ASSERT(x >= -1, "mapTile: x value is too small (%d,%d) in %dx%d",x,y,mapWidth,mapHeight);
	ASSERT(y >= -1, "mapTile: y value is too small (%d,%d) in %dx%d",x,y,mapWidth,mapHeight);
	x = (x < 0 ? 0 : x);
	y = (y < 0 ? 0 : y);
	ASSERT(x < mapWidth + 1, "mapTile: x value is too big (%d,%d) in %dx%d",x,y,mapWidth,mapHeight);
	ASSERT(y < mapHeight + 1, "mapTile: y value is too big (%d,%d) in %dx%d",x,y,mapWidth,mapHeight);
	x = (x >= mapWidth ? mapWidth - 1 : x);
	y = (y >= mapHeight ? mapHeight - 1 : y);

	return &psMapTiles[x + (y * mapWidth)];
}

The way these checks are written, a invalid map with size mapwidth=0 and mapheight=0 as encountered in this network game lead to the check:

	x = (x >= mapWidth ? mapWidth - 1 : x);
	y = (y >= mapHeight ? mapHeight - 1 : y);

calculating the pointer as

	return &psMapTiles[-1];

SEGFAULT!

Though apart from the assertions handling this fatal case incorrectly, a map with 0x0 size shouldn't have been loaded in the first place!

Please someone add a check after map download and parsing.


Issue migrated from trac:2135 at 2022-04-16 06:35:00 -0700

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

corvuscorax@... uploaded file warzone2100.gdmp-S60HD1 (51.9 KiB)

backtrace file

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

Buginator changed status from new to closed

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

Buginator changed resolution from `` to fixed

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

Buginator commented


(In [11585]) Add checks to maps that are too small.

fixes #2135

@wzdev-ci wzdev-ci closed this as completed Sep 3, 2010
@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

Buginator commented


Nice details, thanks, it is appreciated!

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

corvuscorax@... commented


Replying to Warzone2100/old-trac-import#2135 (comment:2):

Nice details, thanks, it is appreciated!

Cool no worries :)

But I think you forgot to put a

free(pFileData);

in map.c line 284

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Sep 3, 2010

svnsync commented


(In [11587]) Missed a free().

Thanks to corvuscorax for the heads-up!
refs #2135

@wzdev-ci
Copy link
Contributor Author

buginator <buginator@...> commented


(In Warzone2100/warzone2100@baab97f) Add checks to maps that are too small.

fixes #2135

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/branches/2.3@11585 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 43570ae1ff39606503ef8c29b8e31cdf1a48af8f)

@wzdev-ci
Copy link
Contributor Author

buginator <buginator@...> commented


In Warzone2100/warzone2100@baab97f:

#CommitTicketReference repository="" revision="baab97f7a9b85bedbaad42eedb8c97073d9baec3"
Add checks to maps that are too small.

fixes #2135

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/branches/2.3@11585 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 43570ae1ff39606503ef8c29b8e31cdf1a48af8f)

1 similar comment
@wzdev-ci
Copy link
Contributor Author

buginator <buginator@...> commented


In Warzone2100/warzone2100@baab97f:

#CommitTicketReference repository="" revision="baab97f7a9b85bedbaad42eedb8c97073d9baec3"
Add checks to maps that are too small.

fixes #2135

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/branches/2.3@11585 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 43570ae1ff39606503ef8c29b8e31cdf1a48af8f)

@wzdev-ci
Copy link
Contributor Author

Buginator commented


Add checks to maps that are too small.

fixes #2135

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/branches/2.3@11585 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 43570ae1ff39606503ef8c29b8e31cdf1a48af8f)
Changeset: baab97f7a9b85bedbaad42eedb8c97073d9baec3

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant