Opinions on the security of my shop system, via RemoteFunctions

I will show both the server and client side to my shop system for my murderer game. I feel like I did a pretty good job making it secure with my remote functions, but I just want your opinion on if there’s anything I should do to improve the security.

What the server does (RemoteFunctions)

RemoteFunctions.Shop.OnServerInvoke = function(player, Signal, Item)
	if player ~= nil then
		if Signal == "Buy" then
			if player.Data:WaitForChild("Cash").Value >= Item.Price.Value then
				player.Data.Cash.Value = player.Data.Cash.Value - Item.Price.Value
				local NewItem = Instance.new("ObjectValue")
				NewItem.Name = Item.Name  
				NewItem.Parent = player.Data:WaitForChild(Item.Parent.Name)
				player.Data:WaitForChild(string.sub(Item.Parent.Name, 1, (string.len(Item.Parent.Name) - 1))).Value = Item.Name
				return true
			end
		elseif Signal == "Equip" then
			if player.Data:WaitForChild(Item.Parent.Name):FindFirstChild(Item.Name) then
				local DataVal = player.Data:WaitForChild(string.sub(Item.Parent.Name, 1, (string.len(Item.Parent.Name) - 1)))
				if DataVal.Value ~= Item.Name then
					DataVal.Value = Item.Name
					return true
				end
			end
		elseif Signal == "Unequip" then
			if player.Data:WaitForChild(Item.Parent.Name):FindFirstChild(Item.Name) then
				local DataVal = player.Data:WaitForChild(string.sub(Item.Parent.Name, 1, (string.len(Item.Parent.Name) - 1)))
				if DataVal.Value == Item.Name then
					DataVal.Value = ""
					return true
				end
			end
		end
	end
	return false
end

This is the client and ui control:

local player = game.Players.LocalPlayer
local leaderstats = player:WaitForChild("leaderstats")
local Data = player:WaitForChild("Data")

local Ui = script.Parent.Main
local Content = game.ReplicatedStorage.Content

local sounds = game.SoundService
local events = game.ReplicatedStorage.Events
local RF = game.ReplicatedStorage.RemoteFunctions

local GameModule = require(game.ReplicatedStorage.Modules.GameModule)
local ViewportModule = require(game.ReplicatedStorage.Modules.ViewportModule)

local ShopContent = game.ReplicatedStorage.Shop

local LastTab = nil
local CurrentTab = nil
local CurrentItem = nil

local MPS = game:GetService("MarketplaceService")

local ShopFrame = Ui:FindFirstChild("ShopFrame")

local DataVal = {"KnifeSkin", "GunSkin", "KnifeEffect", "GunEffect", "KnifePower", "Gear", "Pet", "Perk"}
local TabNames = {"Knife Skins", "Gun Skins", "Knife Effects", "Gun Effects", "Knife Powers", "Gear", "Pets", "Perks", "Buy Cash"}
local Tabs = {ShopFrame.TopFrame.Buttons.KnifeSkins, ShopFrame.TopFrame.Buttons.GunSkins, ShopFrame.TopFrame.Buttons.KnifeEffects, ShopFrame.TopFrame.Buttons.GunEffects, ShopFrame.TopFrame.Buttons.KnifePowers, ShopFrame.TopFrame.Buttons.Gears, ShopFrame.TopFrame.Buttons.Pets, ShopFrame.TopFrame.Buttons.Perks, ShopFrame.TopFrame.Buttons.BuyCash}
local Frames = {ShopFrame.ShopFrameHolder.FrameImage.KnifeSkins, ShopFrame.ShopFrameHolder.FrameImage.GunSkins, ShopFrame.ShopFrameHolder.FrameImage.KnifeEffects, ShopFrame.ShopFrameHolder.FrameImage.GunEffects, ShopFrame.ShopFrameHolder.FrameImage.KnifePowers, ShopFrame.ShopFrameHolder.FrameImage.Gears, ShopFrame.ShopFrameHolder.FrameImage.Pets, ShopFrame.ShopFrameHolder.FrameImage.Perks, ShopFrame.ShopFrameHolder.FrameImage.BuyCash}
local ShopList = {ShopContent.KnifeSkins, ShopContent.GunSkins, ShopContent.KnifeEffects, ShopContent.GunEffects, ShopContent.KnifePowers, ShopContent.Gears, ShopContent.Pets, ShopContent.Perks, ShopContent.BuyCash}

local ItemPositioningGuide = {UDim2.new(0, 15, 0, 0), UDim2.new(0, 101, 0, 0), UDim2.new(0, 188, 0, 0), UDim2.new(0, 274, 0, 0), UDim2.new(0, 360, 0, 0), UDim2.new(0, 446, 0, 0)}
-- note to self position the first 6 item boxes to these udim2 coordinates and make the rest correspond -- GoneCorrupted

local CheckRoles = function()
	if table.find(GameModule.Creators, player.UserId) ~= nil or table.find(GameModule.Admins, player.UserId) ~= nil or table.find(GameModule.Mods, player.UserId) ~= nil or table.find(GameModule.Interns, player.UserId) ~= nil then
		return true
	end
	return false
end

local GetInfo = function(Signal, Item, Type)
	if Signal == "Start" then
		if not Item:FindFirstChild("GamePass") and not Item:FindFirstChild("DevProduct") then
			if RF.CheckInventory:InvokeServer(Item, Type) then
				local ItemEquipped = false 
				local CheckData = Data:GetDescendants()
				for i = 1, #CheckData do
					if CheckData[i]:IsA("StringValue") and CheckData[i].Name == string.sub(Type.Name, 1, (string.len(Type.Name) - 1)) then
						if CheckData[i].Value == Item.Name then
							ItemEquipped = true
						end
					end
				end
				if not ItemEquipped then
					ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Equip"
				elseif ItemEquipped then
					ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Unequip"
				end
			else
				ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Purchase!"
			end
		else 
			ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Purchase!"
		end
		ShopFrame.ItemFrame.ItemInfo.Buy.Visible = true
		if Item:FindFirstChild("GamePass") or Item:FindFirstChild("DevProduct") then
			ShopFrame.ItemFrame.ItemInfo.Price.Text = Item.Price.Value.." Robux"
		else
			ShopFrame.ItemFrame.ItemInfo.Price.Text = Item.Price.Value.." Cash"
		end
		ShopFrame.ItemFrame.ItemInfo.CreatorThumbnail.Creator.Image = game.Players:GetUserThumbnailAsync(Item.Creator.Value, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420)
		ShopFrame.ItemFrame.ItemInfo.CreatorName.Text = "By "..game.Players:GetNameFromUserIdAsync(Item.Creator.Value)
		ShopFrame.ItemFrame.ItemInfo.CreatorThumbnail.Visible = true
		ShopFrame.ItemFrame.ItemInfo.Description.Text = Item.Desc.Value
		ShopFrame.ItemFrame.ItemInfo.ItemName.Text = Item.Name
		ViewportModule.Create3d(Item.Handle, ShopFrame.ViewportFrame, Type)
	elseif Signal == "Stop" then
		ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = ""
		ShopFrame.ItemFrame.ItemInfo.Price.Text = ""
		ShopFrame.ItemFrame.ItemInfo.Buy.Visible = false 
		ShopFrame.ItemFrame.ItemInfo.CreatorThumbnail.Creator.Image = ""
		ShopFrame.ItemFrame.ItemInfo.CreatorName.Text = ""
		ShopFrame.ItemFrame.ItemInfo.CreatorThumbnail.Visible = false
		ShopFrame.ItemFrame.ItemInfo.Description.Text = ""
		ShopFrame.ItemFrame.ItemInfo.ItemName.Text = ""
		if ShopFrame.ViewportFrame:FindFirstChild("Handle") then
			ShopFrame.ViewportFrame.Handle:Destroy()
		end
	end
end

local function GetAssets(Type, Tab, Frame)
	local TableOfAssets = {}
	local Assets = Type:GetChildren()
	for a = 1, #Assets do
		if Assets[a] ~= nil then
			table.insert(TableOfAssets, Assets[a])
		end
	end
	table.sort(TableOfAssets, function(a1, a2)
		if a1:FindFirstChild("Order") then
			return a1.Order.Value < a2.Order.Value
		else
			return a1.Price.Value < a2.Price.Value
		end
	end)
	for t = 1, #TableOfAssets do
		local NewButton = Instance.new("ImageButton")
		NewButton.Name = TableOfAssets[t].Name
		NewButton.BackgroundTransparency = 1
		NewButton.Image = "rbxassetid://11614064509"
		NewButton.Size = UDim2.new(0, 82, 0, 82)
		NewButton.Parent = Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name]
		if t >= 1 then
			if t < 7 then
				NewButton.Position = ItemPositioningGuide[t]
			elseif t >= 7 then -- note to self - Position boxes to boxes on last row 
				local FindFrame = Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name]:GetChildren()
				for x = 1, #FindFrame do
					NewButton.Position = FindFrame[t - 6].Position + UDim2.new(0, 0, 0, 85)
				end
			end
		end
		local Image = Instance.new("ImageLabel")
		Image.Name = "Image"
		Image.Image = "http://www.roblox.com/asset/?id="..TableOfAssets[t].Image.Value
		Image.Size = UDim2.new(0, 53, 0, 56)
		Image.Position = UDim2.new(0.168, 0, 0.15, 0)
		Image.BackgroundTransparency = 1
		Image.ZIndex = 1
		Image.Parent = NewButton
		local Owned = Instance.new("ImageLabel")
		Owned.Name = "Owned"
		Owned.Image = "http://www.roblox.com/asset/?id=11157772247"
		Owned.BackgroundTransparency = 1
		Owned.Size = UDim2.new(0, 27, 0, 33)
		Owned.Position = UDim2.new(0.561, 0, 0.561, 0)
		if Type.Name ~= "BuyCash" then
			if RF.CheckInventory:InvokeServer(TableOfAssets[t], Type) then
				Owned.Visible = true 
			else
				Owned.Visible = false
			end
		else
			if TableOfAssets[t]:FindFirstChild("GamePass") then
				if game.MarketplaceService:UserOwnsGamePassAsync(player.UserId, TableOfAssets[t].GamePass.Value) or CheckRoles() then
					Owned.Visible = true
				else
					Owned.Visible = false
				end
			elseif TableOfAssets[t]:FindFirstChild("DevProduct") then
				Owned.Visible = false
			end	
			-- most likely in the buy cash section
		end
		Owned.ZIndex = 1
		Owned.Parent = NewButton
		local OGCanvasY = Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name]:GetChildren()[1].Size.Y.Offset
		local OGCanvasX = Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name].CanvasSize.X.Offset
		local CanvasYConversion = OGCanvasY + (((math.floor(#Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name]:GetChildren()) / 6) * 85)) - ((math.floor(#Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name]:GetChildren()) / 6) * 6.25)
		Ui.ShopFrame.ShopFrameHolder.FrameImage[Type.Name].CanvasSize = UDim2.new(0, OGCanvasX, 0, CanvasYConversion)
		NewButton.MouseEnter:Connect(function()
			NewButton.ImageColor3 = Color3.new(0.529412, 0.529412, 0.529412)
		end)
		NewButton.MouseLeave:Connect(function()
			NewButton.ImageColor3 = Color3.new(1, 1, 1)
		end)
		NewButton.MouseButton1Click:Connect(function()
			if CurrentItem ~= TableOfAssets[t] then
				sounds.Click:Play()
			end
			CurrentItem = TableOfAssets[t]
			GetInfo("Start", CurrentItem, Type)
		end)
	end
end

for i = 1, #ShopList do
	if ShopList[i] ~= nil then
		GetAssets(ShopList[i], Tabs[i], Frames[i])
	end
end

for x = 1, #Tabs do
	Tabs[x].MouseButton1Click:Connect(function()
		--local BackpackAcquired = false
		CurrentTab = Tabs[x]
		if LastTab ~= CurrentTab then
			LastTab = CurrentTab
			sounds.Click:Play()
			GetInfo("Stop")
			CurrentItem = nil
			ShopFrame.TabName.Label.Text = TabNames[x]
			if ShopFrame.ShopFrameHolder.FrameImage:FindFirstChild("Info") then
				ShopFrame.ShopFrameHolder.FrameImage.Info:Destroy()
			end
			for y = 1, #Frames do
				if y == x then
					Frames[y].Visible = true
				else
					Frames[y].Visible = false 
				end
			end
			local DataType = nil
			if CurrentTab.Name ~= "BuyCash" then
				DataType = Data:WaitForChild(DataVal[x])
			end
			if DataType ~= nil then
				if DataType.Value ~= "" then
					if Frames[x]:FindFirstChild(DataType.Value) then
						CurrentItem = ShopList[x][DataType.Value]
						GetInfo("Start", ShopList[x][DataType.Value], ShopList[x])
						--BackpackAcquired = true 
					end
				end
			end
		end
	end)
	Tabs[x].MouseEnter:Connect(function()
		Tabs[x].ImageColor3 = Color3.new(0.596078, 0.596078, 0.596078)
	end)
	Tabs[x].MouseLeave:Connect(function()
		Tabs[x].ImageColor3 = Color3.new(1, 1, 1)
	end)
end


local function OpenShop()
	if Ui.ShopFrame.Visible == false then
		Ui.ShopFrame.Visible = true
	elseif Ui.ShopFrame.Visible == true then
		Ui.ShopFrame.Visible = false
	end
	sounds["Open/CloseGUISound"]:Play()
end

local function Buy()
	if CurrentItem ~= nil and ShopFrame.ItemFrame.ItemInfo.ItemName.Text == CurrentItem.Name then
		if not CurrentItem:FindFirstChild("GamePass") and not CurrentItem:FindFirstChild("DevProduct") then
			if RF.CheckInventory:InvokeServer(CurrentItem, CurrentItem.Parent) then
				local ItemEquipped = false
				for a = 1, #DataVal do
					if DataVal[a] == string.sub(CurrentItem.Parent.Name, 1, (string.len(CurrentItem.Parent.Name) - 1)) then
						local DataType = Data:WaitForChild(DataVal[a])
						if DataType.Value == CurrentItem.Name or DataType.Value == CurrentItem.Handle.Mesh.TextureId then
							ItemEquipped = true
						end
					end
				end
				if ItemEquipped then
					if RF.Shop:InvokeServer("Unequip", CurrentItem) == true then
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Equip"
					end
				elseif not ItemEquipped then
					if RF.Shop:InvokeServer("Equip", CurrentItem) == true then
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Unequip"
					end
				end
			else
				if Data:WaitForChild("Cash").Value >= CurrentItem.Price.Value then
					if RF.Shop:InvokeServer("Buy", CurrentItem) == true then
						sounds.PurchaseGranted:Play()
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Unequip"
						Ui.ShopFrame.ShopFrameHolder.FrameImage[CurrentItem.Parent.Name][CurrentItem.Name].Owned.Visible = true
					else
						sounds.PurchaseDecline:Play()
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.ImageColor3 = Color3.new(1, 0.337255, 0.278431)
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Declined"
						warn("You've attempted to cheat, cheating in game is prohitbited and can result in a ban!")
						delay(0.5, function()
							Ui.ShopFrame.ItemFrame.ItemInfo.Buy.ImageColor3 = Color3.new(0.333333, 1, 0.498039)
							Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Purchase!"
						end)
					end
				else
					sounds.PurchaseDecline:Play()
					Ui.ShopFrame.ItemFrame.ItemInfo.Buy.ImageColor3 = Color3.new(1, 0.337255, 0.278431)
					Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Declined"
					delay(0.5, function()
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.ImageColor3 = Color3.new(0.333333, 1, 0.498039)
						Ui.ShopFrame.ItemFrame.ItemInfo.Buy.TextLabel.Text = "Purchase!"
					end)
				end
			end
		else
			if CurrentItem:FindFirstChild("GamePass") then
				MPS:PromptGamePassPurchase(player, CurrentItem.GamePass.Value)
			elseif CurrentItem:FindFirstChild("DevProduct") then
				MPS:PromptProductPurchase(player, CurrentItem.DevProduct.Value)
			end
		end
	end
end

Ui.ShopFrame.ItemFrame.ItemInfo.Buy.MouseButton1Click:Connect(Buy)
Ui.Back.Shop.MouseButton1Click:Connect(OpenShop)

Items haven’t dropped in every tab yet but this is how it came out:

2 Likes