Skip to content

Conversation

@tens0rfl0w
Copy link
Contributor

Goal of this PR

This PR addresses two issues: missing bounds checks that allowed reading out-of-bounds memory, and a missing parameter in the native declaration of GET_VEHICLE_DOOR_STATUS.

How is this PR achieving the goal

  • Introduced checks to ensure that array indices are within valid bounds before accessing data, preventing out-of-bounds reads.
  • Updated the native declaration of GET_VEHICLE_DOOR_STATUS to include the missing doorIndex parameter.
  • Corrected array size for doorPositions in CVehicleGameStateNodeData.

This PR applies to the following area(s)

FiveM, Server

Successfully tested on

Game builds: 3258

Platforms: Windows (Server)

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

Fixes issues

Reported here: https://discord.com/channels/779705925577080842/779739477642838036/1304152704137826304 (Cfx.re Engineering Group)

@github-actions github-actions bot added the invalid Requires changes before it's considered valid and can be (re)triaged label Nov 11, 2024
@tens0rfl0w tens0rfl0w force-pushed the fix/server-game-state/array-bounds-check branch from 607f42a to c7444b0 Compare November 11, 2024 15:35
@Nobelium-cfx Nobelium-cfx added the has-dependencies Blocked by or requires another change, read PR description. label Nov 13, 2024
@Nobelium-cfx
Copy link
Contributor

Should be merged after #2925

…tions

This missed a couple of array bounds checks and a missing parameter in the native decl. of "GET_VEHICLE_DOOR_STATUS".
@tens0rfl0w tens0rfl0w force-pushed the fix/server-game-state/array-bounds-check branch from c7444b0 to b10506d Compare November 17, 2024 23:24
@github-actions github-actions bot added triage Needs a preliminary assessment to determine the urgency and required action and removed invalid Requires changes before it's considered valid and can be (re)triaged labels Nov 18, 2024
@Nobelium-cfx Nobelium-cfx removed the has-dependencies Blocked by or requires another change, read PR description. label Nov 25, 2024
Copy link
Contributor

@FabianTerhorst FabianTerhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@FabianTerhorst FabianTerhorst added ready-to-merge This PR is enqueued for merging and removed triage Needs a preliminary assessment to determine the urgency and required action labels Jan 10, 2025
@prikolium-cfx prikolium-cfx merged commit f36bc67 into citizenfx:master Jan 21, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR is enqueued for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants