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

Moving to server side checks for inventory #422

Closed

Conversation

r0adra93
Copy link

@r0adra93 r0adra93 commented Nov 14, 2022

Security Fix for qb-policejob that coincides with inventory security fix located at: qbcore-framework/qb-inventory#380

Questions (please complete the following information):

  • Have you personally loaded this code into an updated qbcore project and checked all it's functionality? [yes/no] (Be honest)
  • Does your code fit the style guidelines? [yes/no]
  • Does your PR fit the contribution guidelines? [yes/no]

Security Fix for qb-policejob that coincides with inventory security fix
Copy link
Contributor

@Irishstevie Irishstevie left a comment

Choose a reason for hiding this comment

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

I run this with qb-policejob: #380

Tested storage Pull out cars open all the menus

Works good

@Z3rio Z3rio self-requested a review December 12, 2022 19:21
@SKITTLE6969
Copy link

Im getting this error after updating this along with the inventory :|
image

@r0adra93
Copy link
Author

Im getting this error after updating this along with the inventory :| image

Is this off a vanilla install of the core or do you have a bunch of addons?

@r0adra93
Copy link
Author

Im getting this error after updating this along with the inventory :| image

I am unable to reproduce on a vanilla install.
I had to update the inventory from github as the vanilla install doesn't include the latest merge.

However, this works as intended and expected with no errors.

Z3rio
Z3rio previously requested changes Dec 16, 2022
Copy link
Contributor

@Z3rio Z3rio left a comment

Choose a reason for hiding this comment

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

Anyone can give themselves all police items just by triggering the police:server:addVehItems event.
Here are the possible fixes for this:

  • Use a callback instead of an event (easiest but not preferred fix)
  • Create a table of all vehicle plates spawned through qb-policejob (someway) and check if the vehicle is in there, or simply check if the plate has the correct syntax
  • Check if the vehicle model is correct (hardest but kinda best one)

@r0adra93
Copy link
Author

Anyone can give themselves all police items just by triggering the police:server:addVehItems event. Here are the possible fixes for this:

  • Use a callback instead of an event (easiest but not preferred fix)
  • Create a table of all vehicle plates spawned through qb-policejob (someway) and check if the vehicle is in there, or simply check if the plate has the correct syntax
  • Check if the vehicle model is correct (hardest but kinda best one)

Qb-jobs resolves all these issues.
I'll reference it to make the changes here.

@Z3rio
Copy link
Contributor

Z3rio commented Dec 16, 2022

Anyone can give themselves all police items just by triggering the police:server:addVehItems event. Here are the possible fixes for this:

  • Use a callback instead of an event (easiest but not preferred fix)
  • Create a table of all vehicle plates spawned through qb-policejob (someway) and check if the vehicle is in there, or simply check if the plate has the correct syntax
  • Check if the vehicle model is correct (hardest but kinda best one)

Qb-jobs resolves all these issues. I'll reference it to make the changes here.

Okay, but you do have to understand that I cant commit a PR with such security issue.

@r0adra93
Copy link
Author

Anyone can give themselves all police items just by triggering the police:server:addVehItems event. Here are the possible fixes for this:

  • Use a callback instead of an event (easiest but not preferred fix)
  • Create a table of all vehicle plates spawned through qb-policejob (someway) and check if the vehicle is in there, or simply check if the plate has the correct syntax
  • Check if the vehicle model is correct (hardest but kinda best one)

Qb-jobs resolves all these issues. I'll reference it to make the changes here.

Okay, but you do have to understand that I cant commit a PR with such security issue.

That's fine i'll pull the trackVeh table, vehCount table and call backs from qb-jobs.

added further security enhancements
@r0adra93 r0adra93 requested a review from Z3rio December 17, 2022 18:55
@r0adra93
Copy link
Author

Anyone can give themselves all police items just by triggering the police:server:addVehItems event. Here are the possible fixes for this:

  • Use a callback instead of an event (easiest but not preferred fix)
  • Create a table of all vehicle plates spawned through qb-policejob (someway) and check if the vehicle is in there, or simply check if the plate has the correct syntax
  • Check if the vehicle model is correct (hardest but kinda best one)

Qb-jobs resolves all these issues. I'll reference it to make the changes here.

Okay, but you do have to understand that I cant commit a PR with such security issue.

I DID ALL OF THIS AND STILL NOT MERGED IN!

@S33G S33G dismissed Z3rio’s stale review January 26, 2023 10:40

Road confirmed this is now resolved

@S33G S33G changed the title Security Fix Moving to server side checks for inventory Jan 26, 2023
@r0adra93 r0adra93 closed this Mar 3, 2023
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

Successfully merging this pull request may close these issues.

4 participants