[UPDATED] ND Framework & addons

Andyyy7666 · December 24, 2021, 12:57am

Hello, this is a framework I’ve been working on for a bit now, It’s not perfect but it will be maintained and updated. This framework is inspired by DOJRP.

Video Preview:

Screenshots

Current addons:

Comming soon:

ND Hospitals (Finished: coming soon.)
ND Blips (Finished: coming soon.)
ND Interaction Menu (Work in progress)
ND Robberies (Work in progress)
ND Phone (Work in progress)
ND MDT (Work in progress)
ND Scoreboard (planned)
ND Inventory (planned)

Download:

Documentation:

Dependency:

OxMySQL

Updates:
1.0

Fixed security vulnerabilities, I still gotta move the money system server-side and a couple more stuff. But for now, it’s pretty solid.

1.1

Moved money system fully server-side, Created a shot spotter and an atm script.

1.2

Full SonoranCAD integration.
Made permission for departments easier to set up.
Added an option to disable the money system.
Added separate permissions for the AOP command.
Added a priority script with the option to start it, stop it, and add a cooldown.
Added an option to choose which department can see the priority cooldown.
Added an option to choose which department can use and see the dark web chat command.
More improvements & bug fixes.

1.3

Bug fixes and optimization.
New commands (view config)

1.4

Bug fixes and optimization.
Changes to the database table to support database sync with SonoranCAD.
More exports (view the fxmanifest.lua or documentation for developers)

AvarianKnight · December 24, 2021, 1:50am

Not going to lie, I’m not entirely sure how this is classified as a framework yet.

The first issue, it’s pretty neat that any player can drop any other player if they wanted to

github.com

Andyyy7666/ND_Framework/blob/dd3062d164e90c6475e5685740caecdb2c2ba3e3/ND_Core/source/server/main.lua#L17-L20

    
      
          RegisterNetEvent("exitGame")

          AddEventHandler("exitGame", function(Player) 

              DropPlayer(Player, "Disconnected using framework.")

          end)

Two main things here, usually frameworks have player objects for two main reasons:

exposing direct db calls on events is not a very good idea, a bad actor can spam the shit out of it and lead to your server crashing.
GetPlayerIdentifier(player, 1) assumes that license will always be on that key, but it isn’t guaranteed.

github.com

Andyyy7666/ND_Framework/blob/dd3062d164e90c6475e5685740caecdb2c2ba3e3/ND_Core/source/server/main.lua#L22-L36

    
      
          RegisterNetEvent("getCharacters")

          AddEventHandler("getCharacters", function()

              local player = source

              local license = GetPlayerIdentifier(player, 1)

              exports.oxmysql:execute("SELECT * FROM characters WHERE license = ?", {license}, function(result)

                  if result then

                      characters = {}

                      for i = 1, #result do

                          temp = result[i]

                          table.insert(characters, {id = temp.character_id, firstName = temp.first_name, lastName = temp.last_name, dob = temp.dob, gender = temp.gender, twt = temp.twt, department = temp.department, cash = temp.cash, bank = temp.bank})

                      end

                      TriggerClientEvent("returnCharacters", player, characters)

                  end

              end)

          end)

Why are you using like here for license, when it should just be a regular check?

github.com

Andyyy7666/ND_Framework/blob/dd3062d164e90c6475e5685740caecdb2c2ba3e3/ND_Core/source/server/main.lua#L63-L67

    
      
          exports.oxmysql:execute("DELETE FROM characters WHERE license LIKE ? AND character_id LIKE ?", {license, character_id}, function(id)

              if id then

                  print(id)

              end

          end)

Basically, every line after this is exploitable

github.com

Andyyy7666/ND_Framework/blob/dd3062d164e90c6475e5685740caecdb2c2ba3e3/ND_Core/source/server/main.lua#L88

    
      
              end)

          end)

          

          if config.shotSpotterEnabled then

              RegisterNetEvent("shotSpotterActive")

              AddEventHandler("shotSpotterActive", function(x, y, z, postal)

                  TriggerClientEvent("shotSpotterReport", -1, x, y, z, postal)

              end)

          end

          

          RegisterNetEvent("bankPay")

          AddEventHandler("bankPay", function(characterid, playerid, amount, playerSending, name)

              local player = source

              local license = GetPlayerIdentifier(player, 1)

              exports.oxmysql:execute("UPDATE characters SET bank = bank - ? WHERE license = ? AND character_id = ?", {amount, license, characterid}, function(bank)

                  if bank then

                      print(bank)

                  end

              end)

              TriggerClientEvent("receiveBank", playerid, amount, playerSending, name)

              TriggerClientEvent("updateMoney", player)

None of these events are checked at all, and they all send whatever arguments they’re provided to every user so a bad actor can fill it with a bunch of invalid data and spam every client.

github.com

Andyyy7666/ND_Framework/blob/main/ND_Core/source/server/commands.lua

------------------------------------------------------------------------
------------------------------------------------------------------------
--			DO NOT EDIT IF YOU DON'T KNOW WHAT YOU'RE DOING			  --
--     							 									  --
--	   For support join my discord: https://discord.gg/Z9Mxu72zZ6	  --
------------------------------------------------------------------------
------------------------------------------------------------------------
local aop = "Unknown" -- aop unknown if it's not set.

-- Aop command events
RegisterNetEvent("registerAop")
AddEventHandler("registerAop", function(aopRegistered)
    local player = source
    if IsRolePresent(player, "ADMIN") then
        aop = aopRegistered
        TriggerClientEvent("setAop", -1, aop)
    else
        DropPlayer(player, "Tried to use a mod menu to change the aop.")
    end
end)

This file has been truncated. show original

marshular · December 24, 2021, 2:03am

damn.

Andyyy7666 · December 24, 2021, 2:17am

I wasn’t aware about most of these issues back when I wrote this. I will continuously work to improve the code and these issues will be fixed. Thank you for the feedback

ROCKY_southpaw · December 24, 2021, 2:41am

I’ll also try to be as constructive with this as I can. So please take this on the chin:

You have a config that contains sensitive data, a bot token, that should never be on the client.
As avarian pointed out, a lot of the events are subject to abuse.
There is a questionable thread checking for permissions and some weird waits following it https://github.com/Andyyy7666/ND_Framework/blob/dd3062d164e90c6475e5685740caecdb2c2ba3e3/ND_Core/source/client/main.lua#L27 You should be checking for this stuff server side and not exposing them on client. You can just use a player joined event or network player active instead of this thread.
Starting cash being on client is a poor idea as well

Andyyy7666 · December 24, 2021, 2:49am

Thank you this will be helpful too

threeclient8 · December 24, 2021, 3:53am

Thank you so much for releasing something like this for FREE!!!

LegacysMaps · December 24, 2021, 3:56am

Looking very cool so far, though I will wait a bit perhaps for another version, pending some of the things mentioned from the other coders here! If there is an update planned sometime soon anyways <3

jerkycub · December 24, 2021, 7:12am

Could you expand a little on this and maybe explain what makes this exploitable and what a fix might look like?

I stepped away from my computer about 6 months ago and haven’t touched it since, until yesterday, after spending about 3 months developing a sever…now I want to get back to it, but now I feel like a chunk of my brain is missing, trying to answer this myself. I know the information is up there, I just need my memory jogged.

AvarianKnight · December 24, 2021, 8:43am

No checks, so the client can give them however much money they want

Linden · December 24, 2021, 8:45am

These events are all networked, so they can be triggered by the client. You can arbitrarily trigger server events and send any arguments you want through, so you can give out money and even kick any player from the server with the events here.

Xd_Golden_Tiger · December 24, 2021, 8:46am

Yeah, @Andyyy7666 There are a ton of stuff that you need to re-look over, There are quite a few bugs within the framework.

Like for an example, When you go to re-select a character, If the Paycheck goes through when you are in the UI, the UI will disapear, and you will be left with nothing just a sky, and you need to relog in order for the framework to work again.

ali_exacute · December 24, 2021, 10:59am

simply , just never trust the clients

try to do everything on server if its possible

MojitoBurrito · December 24, 2021, 12:18pm

Thank you Andy this is very cool! I look forward to seeing the development of this once these security concerns are ironed out!

Zenites_Team · December 24, 2021, 1:37pm

I believe that the safest thing is to create methods ( functions ) for everything, like AddItem, AddMoney, RemoveMoney. These important things should never be network events. And all these methods must belong to an object that can be shared to server-side scripts and then when you go to give money to a player, you must call an event that has checks on why the player is making money, something like:

-- simulating a shop event
RegisterNetEvent("MyShops:SellItem")
AddEventHandler("MyShops:SellItem", item)
  local _source = source

  -- Here you check if the player has the item and then remove from it.
  -- This is one of the possible safeguards that prevents a player from earning money without actually having the item.
  -- {...}
  local player = ND.GetPlayer(_source)
  player.AddMoney(amount)

end)

Andyyy7666 · December 25, 2021, 12:56am

The first issue, it’s pretty neat that any player can drop any other player if they wanted to

ND_Framework/main.lua at dd3062d164e90c6475e5685740caecdb2c2ba3e3 · Andyyy7666/ND_Framework · GitHub

This has now been fixed by checking the source instead.

github.com

Andyyy7666/ND_Framework/blob/41dcfc1cda1667a6f02be25e014b4931ced05334/ND_Core/source/server/main.lua#L69-L73

    
      
          RegisterNetEvent("exitGame")

          AddEventHandler("exitGame", function()

              local player = source

              DropPlayer(player, "Disconnected using framework.")

          end)

Two main things here, usually frameworks have player objects for two main reasons:

exposing direct db calls on events is not a very good idea, a bad actor can spam the shit out of it and lead to your server crashing.

GetPlayerIdentifier(player, 1) assumes that license will always be on that key, but it isn’t guaranteed.

ND_Framework/main.lua at dd3062d164e90c6475e5685740caecdb2c2ba3e3 · Andyyy7666/ND_Framework · GitHub

I’ve now added a check here to make sure that the characters can’t go over the limit. This is now checked on the client and server.

github.com

Andyyy7666/ND_Framework/blob/0776057e588335ce341f4dd9bf7a87a7e85eea96/ND_Core/source/server/main.lua#L92-L115

    
      
          RegisterNetEvent("newCharacter")

          AddEventHandler("newCharacter", function(newCharacter)

              local player = source

              local license = GetPlayerIdentifierFromType("license", player)

              exports.oxmysql:execute("SELECT character_id FROM characters WHERE license = ?", {GetPlayerIdentifierFromType("license", player)}, function(result)

                  if (result) and (config.characterLimit > #result) then

                      exports.oxmysql:execute("SELECT MAX(character_id) AS nextID FROM characters WHERE license = ?", {license}, function(result)

                          if result then

                              character_id = result[1].nextID

                              if not character_id then

                                  character_id = 1

                              else

                                  character_id = character_id + 1

                              end

                              exports.oxmysql:execute("INSERT INTO characters (license, character_id, first_name, last_name, dob, gender, twt, department, cash, bank) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", {license, character_id, newCharacter.firstName, newCharacter.lastName, newCharacter.dateOfBirth, newCharacter.gender, newCharacter.twtName, newCharacter.department, newCharacter.startingCash, newCharacter.startingBank}, function(id)

                                  if id then

                                      TriggerClientEvent("returnNewCharacter", player, id, newCharacter)

                                  end

                              end)

                          end

This file has been truncated. show original

I’ve also added this function where it always gets the license when I use GetPlayerIdentifierFromType("license", player)

github.com

Andyyy7666/ND_Framework/blob/0776057e588335ce341f4dd9bf7a87a7e85eea96/ND_Core/source/server/main.lua#L44-L58

    
      
          function GetPlayerIdentifierFromType(type, source) -- Credits: xander1998, Post: https://forum.cfx.re/t/solved-is-there-a-better-way-to-get-lic-steam-and-ip-than-getplayeridentifiers/236243/2?u=andyyy7666

          	local identifiers = {}

          	local identifierCount = GetNumPlayerIdentifiers(source)

          

          	for a = 0, identifierCount do

          		table.insert(identifiers, GetPlayerIdentifier(source, a))

          	end

          

          	for b = 1, #identifiers do

          		if string.find(identifiers[b], type, 1) then

          			return identifiers[b]

          		end

          	end

          	return nil

          end

Why are you using like here for license, when it should just be a regular check?
ND_Framework/main.lua at dd3062d164e90c6475e5685740caecdb2c2ba3e3 · Andyyy7666/ND_Framework · GitHub

This is now fixed.

github.com

Andyyy7666/ND_Framework/blob/0776057e588335ce341f4dd9bf7a87a7e85eea96/ND_Core/source/server/main.lua#L118-L122

    
      
          RegisterNetEvent("delCharacter")

          AddEventHandler("delCharacter", function(character_id)

              local player = source

              exports.oxmysql:execute("DELETE FROM characters WHERE license = ? AND character_id = ?", {GetPlayerIdentifierFromType("license", player), character_id})

          end)

Basically, every line after this is exploitable
ND_Framework/main.lua at dd3062d164e90c6475e5685740caecdb2c2ba3e3 · Andyyy7666/ND_Framework · GitHub

I will be working on moving the money stuff to the server tomorrow.

None of these events are checked at all, and they all send whatever arguments they’re provided to every user so a bad actor can fill it with a bunch of invalid data and spam every client.

    RegisterNetEvent("registerAop")
    AddEventHandler("registerAop", function(aopRegistered)
        aop = aopRegistered
        TriggerClientEvent("setAop", -1, aop)
    end)

I’ve now added a line to check for the Admin role. This also checks if the player is an admin when the command is triggered, so players using the command without the roles won’t be kicked.

github.com

Andyyy7666/ND_Framework/blob/0776057e588335ce341f4dd9bf7a87a7e85eea96/ND_Core/source/server/commands.lua#L11-L20

    
      
          RegisterNetEvent("registerAop")

          AddEventHandler("registerAop", function(aopRegistered)

              local player = source

              if IsRolePresent(player, "ADMIN") then

                  aop = aopRegistered

                  TriggerClientEvent("setAop", -1, aop)

              else

                  DropPlayer(player, "Tried to use a mod menu to change the aop.")

              end

          end)

Thank you for the feedback this helped fix stuff.

jerkycub · December 25, 2021, 9:07am

Ah, yea, okay…it’s coming back to me now…the server should never just accept values from a client. The server should always be doing the calculating and keeping track of any values . But in the case where the server needs to accept a value from the client, like when one player sends money to another player for example, then the server should first be verifying that the client actually has enough money before updating the database.

I don’t know how this basic logic escaped me… that’s embarrassing…

Death_Anglesrp · December 25, 2021, 3:14pm

will this work for FileZilla because I have my esx server from zap but I’ve been look for something like this

LegacysMaps · December 25, 2021, 10:51pm

? Anything works with FileZilla. FileZilla is just an FTP program for transferring files

LegacysMaps · December 25, 2021, 10:52pm

Awesome work! Is this the only update scheduled in the near future? Or anything else immediately the in the works? As in within a week or two, if so I can wait for the next version. One question I have is if it will be seamless upgrades without losing characters/info in the future upgrades.