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

[Exhumed] Potential OOB with sequence for rat. #710

Open
mjr4077au opened this issue Apr 25, 2023 · 3 comments
Open

[Exhumed] Potential OOB with sequence for rat. #710

mjr4077au opened this issue Apr 25, 2023 · 3 comments
Assignees

Comments

@mjr4077au
Copy link
Contributor

mjr4077au commented Apr 25, 2023

In Raze, we've recently just re-written the sequence system to use proper objects to represent a sequence, a sequence's frames, and a frame's chunks. Its a lot better and easier to work with, however so far I've observed a number of OOBs from this, which may or may not be consequential but I wanted to share them anyway.

ZDoom/Raze@fe356f3 repairs an issue where the frame index is XOR'd by 1, resulting in a frame index of 5. The total frames for this sequence is 5, so the net result is the first frame of the next sequence would be played here.

ZDoom/Raze@4428ea5 repairs an issue where for the spider, frames 9 and above do not have any chunks, therefore accessing the chunk array could either return garbage data, or the picnum for another frame or sequence. The fix in this commit is average, it's fixed in the next example.

ZDoom/Raze@1162954 moves the null check into our texture getter to eliminate the potential null accesses, but highlights that I did indeed experience a null access when attempting to get the texture/picnum from a non-existent chunk for the rat, just as I did with the spider.

ZDoom/Raze@ba8bdf3 resets the nFrame value for the Anubis actor where the action was changing, but the frame number was not resetting along with it, as all other nAction changes are doing.

ZDoom/Raze@408c715 was a manual comb through the soruce after fixing the Anubis to find other such occurences, at least 20+ were found.

@mjr4077au
Copy link
Contributor Author

mjr4077au commented Apr 26, 2023

I just patched another super nasty one in ZDoom/Raze@ea7fe0e for skulstrt.seq. There's one sprite face that only has 20 frames instead of 24 like the others.

image

@sirlemonhead
Copy link
Collaborator

Thanks! I'll take a look soon hopefully :)

@sirlemonhead sirlemonhead self-assigned this Apr 26, 2023
@mjr4077au
Copy link
Contributor Author

No worries at all. I know these technically aren't OOBs with the original setup where all frames from all seqs are just in one big array, but it would still lead to undesirable outcomes of the wrong stuff animating, etc

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

2 participants