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

Out of bounds array access #141

Closed
wzdev-ci opened this issue Nov 14, 2008 · 15 comments
Closed

Out of bounds array access #141

wzdev-ci opened this issue Nov 14, 2008 · 15 comments
Labels
closed component: other major

Comments

@wzdev-ci
Copy link
Contributor

resolution_fixed type_bug | by Giel

From [5014] and onward we access the array [doxygen:_droid_template DROID_TEMPLATE]->asParts out of its bounds.

This is because that's the first revision to actually access asParts[COMP_WEAPON]. This while asParts is defined as being DROID_MAXCOMP elements large.

And DROID_MAXCOMP has been defined to (COMP_NUMCOMPONENTS - 1). This is fine if you want to determine the highest component number, which it's name suggests that it does, but it's not fine if want to know the number of components.

So there are two issues involved that need to be fixed:

  • Array size of asParts (and asBits of [doxygen:DROID]) being too small
  • The savegame having these two arrays two short as well; thus it will be difficult to prevent breaking of backwards compatibility

Thus using DROID_MAXCOMP to decide on the size of the asParts array is wrong. Unfortunately, however, this specific wrong usage of DROID_MAXCOMP is used in the save games as well, additionally it is used in map.c too.

Issue migrated from trac:141 at 2022-04-15 17:47:18 -0700
@wzdev-ci
Copy link
Contributor Author

Per commented

I think I looked at this some time, and decided it should be fixed one day, but was not critical. I suggest it is only fixed in trunk, and that the usual savegame versioning is used to keep backwards savegame compatibility, even though I know this is really painful.

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Nov 14, 2008 •
edited

Giel commented

Replying to Warzone2100/old-trac-import#141 (comment:1):

I think I looked at this some time, and decided it should be fixed one day, but was not critical. I suggest it is only fixed in trunk, and that the usual savegame versioning is used to keep backwards save game compatibility, even though I know this is really painful.

Considering that asParts[COMP_WEAPON] is only ever written to and never read, should we perhaps just comment the line where it's written to out in [milestone:2.1]? While still looking for a better fix for trunk.

@wzdev-ci
Copy link
Contributor Author

Giel changed milestone from 2.1 to 2.2

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented Nov 15, 2008 •
edited

Giel commented

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

Considering that asParts[COMP_WEAPON] is only ever written to and never read, should we perhaps just comment the line where it's written to out in [milestone:2.1]? While still looking for a better fix for trunk.

Ahum, cough, considering that this bug got introduced in [5014] and the [milestone:2.1] branch got created in [3812] this particular issue doesn't exist in [milestone:2.1].

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed status from new to closed

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed resolution from `` to fixed

@wzdev-ci
Copy link
Contributor Author

DevUrandom commented

(In [6523]) Another attempt against #141 (out of bounds access at scriptfuncs:11467)

Since the original assignment probably happened out of confusion what asParts[] shall or shall not contain,
I removed the offending line and added a bit more documentation about that array.

fixes #141

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed status from closed to reopened

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed resolution from fixed to ``

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed status from reopened to accepted

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed owner from `` to DevUrandom

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed status from accepted to closed

@wzdev-ci
Copy link
Contributor Author

DevUrandom changed resolution from `` to fixed

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented May 8, 2010

Buginator removed milestone (was 2.2)

@wzdev-ci
Copy link
Contributor Author

wzdev-ci commented May 8, 2010

Buginator commented

Milestone 2.2 deleted

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
closed component: other major
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@wzdev-ci