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

Suspicious memory access #34

Open
xavierh opened this issue Dec 8, 2017 · 5 comments
Open

Suspicious memory access #34

xavierh opened this issue Dec 8, 2017 · 5 comments

Comments

@xavierh
Copy link

xavierh commented Dec 8, 2017

byte partNumber = reader.ReadByte();
parts = new ItemValue[4];
if (partNumber != 0) {
for (int i = 0; i < (int)partNumber; i++) {

Bare in mind that I do not know C# at all.
parts array is initialized with a size of 4 but can potentially go beyond 4

(I was having a look at the code to implement new feature in https://github.com/nicolas-f/7DTD-leaflet)

@Karlovsky120
Copy link
Owner

You can only have 4 parts, but you can have any number of accessories. At least I think so.

Is there an item in the game that has more than 4 parts and can be put in inventory semi-assembled (with more than 4 parts assembled)?

I haven't really played the game much lately.

@madmunki
Copy link
Contributor

madmunki commented Dec 9, 2017

The array is zero indexed. So 0-4 is five. 4 parts, and a flashlight.

@Karlovsky120
Copy link
Owner

Wrong.

0-3 iz four since 4 is excluded (notice the <, not <=).

Accessories have a separate array they're stored in.

The code is fine, unless there is a new part cap that is greater than 4.

@xavierh
Copy link
Author

xavierh commented Dec 11, 2017

The point is, you are assuming that partNumber is between 0-3 (which it should) but it could be anything else as it is read from the file and you potentially would do an index out of bound access. You can not assume that partNumber is between 0 and 3.

Line 49 should be:
for (int i = 0; i < (int)partNumber && i < 4; i++) {

@Karlovsky120
Copy link
Owner

Sure, it could be changed. But if the number really is greater or equal than 4, something is off with the file and the program will fail reading it one way or another. You get an error either way.

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

No branches or pull requests

3 participants