Is sending instances to the server through remotes safe in circumstances like this?

Help and Feedback Scripting Support
,
1

I understand that sending instances through RemoteEvents is generally discouraged due to potential safety concerns or errors. Still, I’ve found myself in a situation where I’m unsure if it’s the most efficient solution or not.

I’m building a custom item pickup system that doesn’t use proximity prompts, click detectors, or character touch detection. This is intentional for how my system is designed. A client sided script has its own way of detecting which item the player is targeting and intends to pick up. This means the choice of which item to pick up is handled entirely on the client.

Here’s the goal:
A local script detects which item (a model in Workspace) the player is trying to pick up. It then tells the server which item it is, so the server can delete said item, and attribute it to the player (aka, make the player “collect” the item).

My direct and simple idea for a solution:
(In this context, the term “Instance” refers to the model that the player aims to pick up, located in the workspace, representing an item.)
My idea is to have the client detect the model and send it directly to the server via a RemoteEvent, basically saying:

“[This player] wants to pick up [this instance].”

  • If the instance doesn’t exist, the server ignores it.

  • If the client modified it, no issue. The server would use the original, server-side instance, since any changes made to it on the client would not be replicated.

  • The server then deletes the model and assigns the corresponding item to the player.

The dilemma I’m facing:
Scripting-wise, this works fine and should be easy to achieve. My concern is purely theoretical. Is this approach safe enough?

Here is an alternative I’ve considered:
If sending the instance directly is unsafe, I could build a server-side “registry”. This would be a table of all item models and their unique IDs. The client would then, grab the ID of the model/item the player is targeting, and send its ID only to the server. The server would then reference the registry to find and detect the correct model.
Basically:

“[This player] wants to pick up item with [this ID]”

  • The server checks the list of instances, and uses the ID received from the client to find which specific model that player chose.

This second registry idea seems more reliable and study, but it adds a lot of complexity. From what I’ve seen online (and I’ve seen a lot), people have very mixed feelings on the topic of sending instances via remotes. Some say it’s safe if you take the precautions, some say it’s something you should absolutely never, ever do! That said, in my case, is it worth the extra effort to make a registry for all “grabbable” items currently in workspace, and use IDs so the client can specify to the server which item it wants, or can I safely just have the client send the instance if I do proper server-side validation?

Some extra things to be taken into consideration:
What am I aware of?

  • That with proper server-side validation, this approach should, in theory, be safe, but I’m not entirely confident, which is why I’m seeking advice.

  • That client-made instances won’t replicate to the server, and the server will receive nil from the remote. That said, the client can’t fake items.

Extra potential risks I’m aware of:

  • Exploiters might attempt to pick any item instantly. This can be mitigated by checking distance between the player and the target item on the server.

Edit: It is also important to clarify this isn’t an “item giver”, but a proper item collection system. Multiple identical or different items may be scattered across the workspace, so it’s imperative that the server identifies the exact model the client is targeting.

TL;DR:
I want the client to send an item instance to the server when picking it up. It works, but I’m unsure if it’s safe even with validation. The safer alternative is attributing item IDs to each item and sending that instead, but it’s more work. I’m on the fence between simplicity and security, and unaware of potential downsides for either approaches.

Edit + Solution:
The second solution, assigning unique IDs for each item, and building a “registry” that references the models using said IDs, is in fact more efficient and secure.
As pointed out by user “d05dev”, sending the instance from the client to the server through a remote can result in reference leaks, instance spoofing, denial of service, race conditions, and other issues, and should therefore not be done.

1 Like
2

You answered your own question. Item IDs are the way to go, store the item id collection in a module script on replicated storage so both client and server can use them.

1 Like
3

To answer your question, no it isn’t safe or secure to take instances from the client. Never trust any input that comes from the client.

Even if you perform some sophisticated client/server side validation, it can still fail, opening the door for advanced attackers to perform injection exploits.

If sending the instance directly is unsafe, I could build a server-side “registry”. This would be a table of all item models and their unique IDs. The client would then, grab the ID of the model/item the player is targeting, and send its ID only to the server. The server would then reference the registry to find and detect the correct model.
Basically:

“[This player] wants to pick up item with [this ID]”

  • The server checks the list of instances, and used the ID received from the client to find which specific model that player chose.

Yes, you should always be doing this. Always keep the business logic and source of truth in control of the server, never the client.


-- Create a ModuleScript in ServerScriptService (ideally in a folder called Data)

local ItemDatabase = {
	healthPotion = {
		id = "minorHealthPotion",
		name = "Minor Health Potion",
		toolTip = "Restores 50 health",
		price = 40
	},
	manaPotion = {
		id = "minorManaPotion",
		name = "Minor Mana Potion",
		toolTip = "Restores 50 mana",
		price = 60
	},
	smallShield = {
		id = "smallShield",
		name = "Small Shield",
		toolTip = "Grants +25 defense",
		price = 200
	},
	trainingSword = {
		id = "trainingSword",
		name = "Training Sword",
		toolTip = "Deals 8-15 damage",
		price = 330
	},	
}

return ItemDatabase


-- elsewhere:
local ItemDatabase = require(game:GetService("ServerScriptService"):WaitForChild("Data"):WaitForChild("ItemDatabase"))

local function onPlayerItemRequest(player, itemId)
	local item = ItemDatabase[itemId]
	-- do something with item
end


YourCustomRemoteEvent.OnServerEvent:Connect(function(player, itemId)
	if not itemId then warn("ItemId was null for player ", player) return end  
        -- Additional itemId typechecking validation here

	onPlayerItemRequest(player, itemId)
end)

Just use SetAttribute for ItemId on the model, then have the client call GetAttribute to get the value of the ItemId attribute on the model that it’s trying to touch, and pass in that ItemId string to the Remote Event.

1 Like
4

Thank you for addressing my security concerns directly. I was unaware that receiving instances from the server can fail and open the door for injection exploits by more advanced attackers.
You’ve also sparked my curiosity. If you don’t mind, is there a chance you elaborate on what types of vulnerabilities might still arise from that even with proper server validation in place? The information I’ve found online about the issues of sending instances through remotes is quite limited.

Regarding the code example you provided, fundamentally it should operate the same way, even with multiple models representing the same item, correct? My goal is to create a system where multiple instances of the same item can spawn in the workspace, which the players would be able to collect, meaning I’d need to register each one in the table individually, even if two items are defined as “the same”. Just to confirm, in this case, that would till be the ideal approach, right? I’m assuming that the essential factor here is maintaining a reliable source of truth on the server to verify the items that the player can collect.

1 Like
5

Thank you for addressing my security concerns directly. I was unaware that receiving instances from the server can fail and open the door for injection exploits by more advanced attackers.

Sure thing.

You’ve also sparked my curiosity. If you don’t mind, is there a chance you elaborate on what types of vulnerabilities might still arise from that even with proper server validation in place?

Off the top of my head, you can have reference leaks, instance spoofing, denial of service, race conditions, things like that.

The problem is the notion of “proper” server validation, in many cases we may think we’re validating properly, but any time we’re writing our own security code, there’s always a chance we have a bug that can let something through.

Regarding the code example you provided, fundamentally it should operate the same way, even with multiple models representing the same item, correct? My goal is to create a system where multiple instances of the same item can spawn in the workspace, which the players would be able to collect, meaning I’d need to register each one in the table individually, even if two items are defined as “the same”.

No you don’t want to duplicate your data. You only need to add each individual item to your table once, like how a word appears once in a dictionary. Then you can reference that one item as many times as needed.

For tracking the touching/picking up of multiple items of the same type in the workspace, you can handle that a few different ways.

I would add an attribute called “itemType” (to avoid confusing with itemId that’s used in the database to look up the item, but you can call it whatever you want) with the item’s type or name, and another attribute called “uniqueId” that you would add at the time of model creation.

(Models actually do have uniqueIds on them that you might be able to access, although I’ve never used them because I always just generated and set my own GUIDs with HttpService:GenerateGuid().)

So now you have the itemType and uniqueId that can be passed into the remoteEvent. ItemType would be used by the server to look up the item, and uniqueId is used to identify the model in the workspace.

One small problem though:

You’re now relying on a model to fire a remote event when a player touches it, however remote events are visible to exploiters and can be fired at any time. So all an attacker would have to do is fire the remote event via console with the type and ID to defeat that security.

You’ll want to add ‘Touched’ event logic to perform validation there.

You can do away with remote events if you put the models under the server ownership. But if you must clone them to the individual client, you’ll still want to perform ‘Touched’ validation in addition to any remote events firing.

In your case though, it sounds like the items in the map are going to be visible for all players to touch? Then you could keep them server-side and just perform logic based on the Touched event trigger.

Just to confirm, in this case, that would till be the ideal approach, right? I’m assuming that the essential factor here is maintaining a reliable source of truth on the server to verify the items that the player can collect.

My rule of thumb when performing data engineering is to keep the approach as simple, secure, and effective as possible for it to work as intended. You’re on the right track.

1 Like
6

Understood.
Thank you for the thorough explanation.
How should the server go about using the UniqueId in order to identify the model in the workspace? Another table containg UniqueIds and their respective models present in the workspace? Or is there another method I haven’t thought of?

1 Like
7

Thank you for the thorough explanation.

Anytime.

local function getModelByUniqueId(uniqueId)
    for _, model in ipairs(workspace:GetDescendants()) do
        if model:IsA("Model") and model:GetAttribute("uniqueId") == uniqueId then
            return model
        end
    end
    return nil
end

You could cache the ID’s to a table if you wanted:

local uniqueIdToModel = {}

-- When you create a model:

model:SetAttribute("uniqueId", generatedUniqueId)

uniqueIdToModel[generatedUniqueId] = model

-- When you need to look up:

local model = uniqueIdToModel[uniqueId]

…although I personally would just use the getModelById function.

1 Like
8

Happy to know I was on the right track! I will be caching them to a table, in order to better manage my server-sided model lookups!
Thank you for your time.
Godspeed, Mr Dev!

Edit: I should also note that my choice of caching them to a table (the second code example) should speed up the lookup process, assuring the lookup time is constant instead of depending on the amount of models present in the workspace.

1 Like
9

That indeed did end up being my solution, as it is more reliable and secure, despite the extra effort. Additionally, you have helped solidify my choice! Thank you!

1 Like
10

In that scenario caching does make sense, especially if you end up with hundreds/thousands of items. Glad we found your solution! :metal:

1 Like
11

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.