Skip to content
Merged
2 changes: 2 additions & 0 deletions lua/autorun/wire_load.lua
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ if SERVER then
AddCSLuaFile("wire/client/e2helper.lua")
AddCSLuaFile("wire/client/e2descriptions.lua")
AddCSLuaFile("wire/client/e2_extension_menu.lua")
AddCSLuaFile("wire/client/e2_viewrequest_menu.lua")
AddCSLuaFile("wire/client/gmod_tool_auto.lua")
AddCSLuaFile("wire/client/sound_browser.lua")
AddCSLuaFile("wire/client/thrusterlib.lua")
Expand Down Expand Up @@ -120,6 +121,7 @@ if CLIENT then
include("wire/client/e2helper.lua")
include("wire/client/e2descriptions.lua")
include("wire/client/e2_extension_menu.lua")
include("wire/client/e2_viewrequest_menu.lua")
include("wire/client/gmod_tool_auto.lua")
include("wire/client/sound_browser.lua")
include("wire/client/thrusterlib.lua")
Expand Down
139 changes: 139 additions & 0 deletions lua/wire/client/e2_viewrequest_menu.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
local function AnswerRequest(accepted, initiator, chip)
net.Start("WireExpression2_AnswerRequest")
net.WriteUInt(accepted, 8)
net.WriteEntity(initiator)
net.WriteEntity(chip)
net.SendToServer()
end

local viewRequests = {}

-- Validates a single request using the initiator and chip (very similar to the server side equivalent in expression2.lua)
local function ValidateRequest(initiator, chip)
if not viewRequests[initiator] or not viewRequests[initiator][chip] then return false end -- Initiator either has no data in viewRequests or has no request for this chip
if not IsValid(initiator) then -- Invalid initiator in request table
viewRequests[initiator] = nil
return false
end
if not IsValid(chip) or chip:GetClass() ~= "gmod_wire_expression2"or CurTime() > viewRequests[initiator][chip].expiry then -- Invalid chip in request table or expired
viewRequests[initiator][chip] = nil
return false
end
return true
end

net.Receive("WireExpression2_ViewRequest", function()
local initiator, chip, name, expiry = net.ReadEntity(), net.ReadEntity(), net.ReadString(), net.ReadFloat()
if not viewRequests[initiator] then viewRequests[initiator] = {} end -- Initialise this user in the viewRequests table if not in there already
viewRequests[initiator][chip] = { name = name, expiry = expiry }
end)

list.Set("DesktopWindows", "WireExpression2_ViewRequestMenu", {
title = "View Requests",
icon = "beer/wiremod/gate_e2", -- Use whatever icon you want here, I just picked my favourite out of the available E2 ones
init = function(icon, window)
local container = vgui.Create("DFrame")
container:SetTitle("Expression 2 View Requests")

container:SetSize(ScrW() * 0.3, ScrH() * 0.6)
container:SetSizable(true)
container:SetMinWidth(ScrW() * 0.1)
container:SetMinHeight(ScrH() * 0.2)

container:Center()
container:MakePopup()

local reqList = vgui.Create("DListView", container)
reqList:Dock(FILL)
reqList:SetMultiSelect(false)

reqList:AddColumn("ID")
reqList:AddColumn("Requested By")
reqList:AddColumn("E2 Name")
reqList:AddColumn("Expires In")

for initiator, requests in pairs(viewRequests) do
for chip, request in pairs(requests) do
if ValidateRequest(initiator, chip) then
local line = reqList:AddLine(tostring(chip:EntIndex()), initiator:Nick(), request.name, tostring(math.ceil(request.expiry - CurTime())))
line.initiator = initiator
line.chip = chip
end
end
end

local frameCounter = -1
function reqList:Think()
-- We don't want to do this EVERY frame, in case there are a bunch of requests, so just modulo a counter and refresh on 0
frameCounter = (frameCounter + 1) % 10 -- Frame dependant can cause some issues with both super high and low FPS, however wont really affect this
if frameCounter ~= 0 then return end

local displayed = {}
for k, line in pairs(self:GetLines()) do
if not ValidateRequest(line.initiator, line.chip) then
self:RemoveLine(k)
else
line:SetColumnText(4, tostring(math.ceil(viewRequests[line.initiator][line.chip].expiry - CurTime())))
if not displayed[line.initiator] then displayed[line.initiator] = {} end
displayed[line.initiator][line.chip] = true
end
end

for initiator, requests in pairs(viewRequests) do
for chip, request in pairs(requests) do
if not displayed[initiator][chip] and ValidateRequest(initiator, chip) then
local line = self:AddLine(tostring(chip:EntIndex()), initiator:Nick(), request.name, tostring(math.ceil(request.expiry - CurTime())))
line.initiator = initiator
line.chip = chip
end
end
end
end

function reqList:OnRowRightClick(id, line)
local mnu = DermaMenu()

if not ValidateRequest(line.initiator, line.chip) then
self:RemoveLine(id)
return
end

mnu:AddOption("Accept Once", function()
local confirm = Derma_Query(
"Are you SURE you want "..line.initiator:Nick().." to have complete access to the code in your chip '"..viewRequests[line.initiator][line.chip].name.."'?\nThis means they are able to steal and redistribute it, so you should only do this if you are certain you can trust them",
"Confirm",
"Yes", function()
if ValidateRequest(line.initiator, line.chip) then
AnswerRequest(1, line.initiator, line.chip)
self:RemoveLine(id)
viewRequests[line.initiator][line.chip] = nil
end
end,
"No", function() end
)
end)
mnu:AddOption("Accept Always", function()
local confirm = Derma_Query(
"Are you SURE you want "..line.initiator:Nick().." to have complete access to the code in your chip '"..viewRequests[line.initiator][line.chip].name.."' for the duration the chip entity exists?\nThis means they are able to steal and redistribute it, as well as view any modifications you make to the chip, so you should only do this if you are certain you can trust them",
"Confirm",
"Yes", function()
if ValidateRequest(line.initiator, line.chip) then
AnswerRequest(2, line.initiator, line.chip)
self:RemoveLine(id)
viewRequests[line.initiator][line.chip] = nil
end
end,
"No", function() end
)
end)
mnu:AddOption("Reject", function()
if ValidateRequest(line.initiator, line.chip) then
AnswerRequest(0, line.initiator, line.chip)
self:RemoveLine(id)
viewRequests[line.initiator][line.chip] = nil
end
end)
mnu:Open()
end
end
})
194 changes: 173 additions & 21 deletions lua/wire/stools/expression2.lua
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ TOOL.ClientConVar = {
TOOL.MaxLimitName = "wire_expressions"
WireToolSetup.BaseLang()

-- Needed a method for printing to players' chatboxes without the outdated and limited umsg based ChatPrint()
-- Not sure if there's already a framework for this in wire, if so replace this with that
local BetterChatPrint = function() end
if SERVER then
util.AddNetworkString("WireExpression2_BetterChatPrint")
BetterChatPrint = function(plr, msg)
net.Start("WireExpression2_BetterChatPrint")
net.WriteString(msg)
net.Send(plr)
end
else
-- Netmsg is coming from the server so no need for sanity checks as the server *should* be as expected unlike clients
net.Receive("WireExpression2_BetterChatPrint", function()
chat.AddText(net.ReadString())
end)
end

if SERVER then
CreateConVar('sbox_maxwire_expressions', 20)

Expand Down Expand Up @@ -55,21 +72,155 @@ if SERVER then
end
end

util.AddNetworkString("WireExpression2_OpenEditor")
function TOOL:RightClick(trace)
if trace.Entity:IsPlayer() then return false end
local bypassModeCVar = CreateConVar(
"wire_expression2_viewrequest_bypass", 1, {FCVAR_ARCHIVE, FCVAR_NOTIFY},
"Sets the admin bypass mode for E2 view requests\n0 - No one can bypass\n1 - Superadmins can bypass (default)\n2 - Superadmins and admins can bypass"
)
local function CheckBypass(plr)
local bypassMode = bypassModeCVar:GetInt()

local player = self:GetOwner()
-- Need the or between IsAdmin and IsSuperAdmin as superadmins may not count as admins due to certain addons
return (bypassMode == 1 and plr:IsSuperAdmin()) or (bypassMode == 2 and (plr:IsAdmin() or plr:IsSuperAdmin()))
end

if IsValid(trace.Entity) and trace.Entity:GetClass() == "gmod_wire_expression2" then
self:Download(player, trace.Entity)
return true
-- Simple serverside only local table for storing view requests to make handling them not spaghetti code
local viewRequests = {}

util.AddNetworkString("WireExpression2_ViewRequest")
util.AddNetworkString("WireExpression2_AnswerRequest")

-- Validates a single request using the initiator and chip (handles cleanup and expiry message)
local function ValidateRequest(initiator, chip)
if not viewRequests[initiator] or not viewRequests[initiator][chip] then return false end -- Initiator either has no data in viewRequests or has no request for this chip
if not IsValid(initiator) then -- Invalid initiator in request table
viewRequests[initiator] = nil
return false
end
if not IsValid(chip) or chip:GetClass() ~= "gmod_wire_expression2" then -- Invalid chip in request table
viewRequests[initiator][chip] = nil
return false
end
if CurTime() > viewRequests[initiator][chip].expiry then -- Expiry point passed
BetterChatPrint(initiator, "Your request to view "..chip.player:Nick().."'s chip, '"..chip.name.."', has expired")
viewRequests[initiator][chip] = nil
return false
end
return true
end

net.Start("WireExpression2_OpenEditor") net.Send(player)
return false
local function InvalidateRequests()
local count = 0
for initiator, _ in pairs(viewRequests) do
for chip, _ in pairs(viewRequests[initiator]) do
if ValidateRequest(initiator, chip) then count = count + 1
elseif not viewRequests[initiator] then break end -- If that validation removed the entire initiating player, stop trying to enumerate their requests
end
end

-- If the count is 0 then there's no requests to invalidate, so remove the hook
if count == 0 then hook.Remove("Tick", "WireExpression2_InvalidateRequests") end
end

local function RequestView(chip, initiator)
local index = chip:EntIndex()
local truncName = string.sub(chip.name, 1, 256) -- In case someone starts making cursed names

-- Make sure this isn't creating a request for a chip with an outstanding valid request
if ValidateRequest(initiator, index) then -- Note that ValidateRequest also deletes the invalid request and handles the expiry notif
BetterChatPrint(initiator, "Request to view '"..truncName.."' already sent")
return
end

-- Otherwise, print to the tool user's chat that a view request was sent
-- and send a view request to the chip's owner (also print a message to their chat to tell them they received a request)
BetterChatPrint(initiator, "E2 view request sent for '"..truncName.."' owned by "..chip.player:Nick())
BetterChatPrint(chip.player, "You just received a request to view your E2 '"..truncName.."' from "..initiator:Nick()..", which you can view in your context menu at the top left ('C' by default)")

-- Add the request data to the local requests table for when the request from the client comes in
if not viewRequests[initiator] then viewRequests[initiator] = {} end -- Initialise this user in the viewRequests table if not in there already
viewRequests[initiator][chip] = {
name = truncName,
expiry = CurTime() + 60 -- 1 minute for the request before it's invalidated (could make this a convar)
}
if not hook.GetTable().Tick.WireExpression2_InvalidateRequests then -- If there's no invalidation hook added, create it now we have requests to invalidate
hook.Add("Tick", "WireExpression2_InvalidateRequests", InvalidateRequests)
end

net.Start("WireExpression2_ViewRequest")
net.WriteEntity(initiator) -- The player attempting to view the E2
net.WriteEntity(chip) -- Chip entity for validation clientside
net.WriteString(truncName) -- Name of the E2 so the owner knows what they're agreeing to
net.WriteFloat(viewRequests[initiator][chip].expiry) -- For making requests expire on time clientside
net.Send(chip.player)
end

util.AddNetworkString("WireExpression2_OpenEditor")
function TOOL:Think()
--[[
I had to replace TOOL:RightClick with TOOL:Think as prop protection was preventing
the view requests system from functioning as intended

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should have been a big hint to you early on that you're doing it wrong...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well if it requires you to give full prop protection perms on something to use view requests, kinda defeats the point.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you rather him overload the gmod_tool functions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of making this a tool option how about using the context menu system so you don't need to potentially break the tool itself for this or have to make another tool, which would be whitelisted. I don't know if FPP can whitelist these actions so you should check this first.

I would personally even want to have something like e2_viewchip in the console for admins.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The view requests almost completely replace the flow for both tool and remote updater code downloading, so I’m not sure what you mean by a tool option or the requirement for an additional tool.


So this manually handles right click meaning people don't need to give each other
full prop protection permissions in order to share a chip
]]
if not IsFirstTimePredicted() then return end

local player = self:GetOwner()
if player:KeyPressed(IN_ATTACK2) then
local chip = player:GetEyeTrace().Entity
if chip:IsPlayer() then return end

local player = self:GetOwner()

if IsValid(chip) and chip:GetClass() == "gmod_wire_expression2" then
if chip.player == player then -- Just download if the toolgun user owns this chip
self:Download(player, chip)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

waitwaitwait, so basically what you're doing here is break prop protection in order to implement your thing?
big fuckin no!
We don't want wiremod to have PP bypass attack surface, ever.
Remove this, think of something else, work within the system.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you mean the bypassList, this was added due to people requesting a method for server operators to be able to take code without permissions to check for malicious chips, this is a server side concmd that takes vararg STEAM_0 ids allowing server owners to give players capable of finding said malicious code a method to do so.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not just the bypassList. I mean this entire function. This cannot be a Think hook since no PP will catch this. They will have to write custom code to deal with E2 and that'd mean even more attack surface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well that's what view requests handles, there is no "attack surface" here, and I actually use a CanTool call as a fallback (forgot to do that until Divran pointed it out though).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"there is no attack surface here" are some famous last words.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well I can require anyone added by server operators (given this is a server only command that sets this list, only people with sv access can set it) to also be an admin, but given a server operator sets it I don’t see the point.

As for allowing a wide range of people access to bypass the view request system (using role based permissions as opposed to the person by person permissions I added) doesn’t really fit with what people should actually be able to take code without permission, given the reason for someone taking code without permission would be to check for malicious code, then a player by player whitelist is required as you cannot expect every admin or above to have the skill required to do so, however you can expect quite a few (enough to warrant not giving ALL of them a bypass anyway) of them to steal code for the purpose of using it themselves or redistributing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let me put it bluntly:
We want server owners to be able to allow their abusive admins to steal your code.
I think there's agreement about this among all the wire team members who weighed in on this so far and you still seem to try and negotiate about that.
What rights you want to give to whom is irrelevant here, Wiremod is just to provide the tools for the server owner to do so, in conjunction with a prop protection addon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We want server owners to be able to allow their abusive admins to steal your code.
I think there's agreement about this among all the wire team members who weighed in on this so far and you still seem to try and negotiate about that.

@TomyLobo are you sure you want to lump the entire wire team in with you on that?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We want server owners to be able to allow their abusive admins to steal your code.

nice

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We want server owners to be able to allow their abusive admins to steal your code.

That's pretty blunt, but I mean yeah pretty much.

At least personally, as a naughty vigilent admin, I'd be editing my copy of Wiremod to allow me to continue to skim through E2's if the consensus was to merge a version of this without built-in admin override. As this PR currently is, I guess I'd be whitelisting myself and all my admins, and then give it a shot - the prompt idea seems cool, might get a bit annoying if you have to keep approving the same friend though. I could see making a convar for enabling/disabling this system (for servers that want to keep the status-quo), though I'd imagine you'd prefer your system to be default-on to "protect against bad admins".

At one point I did indeed write something to save all uploaded E2's to the server, and debated uploading the whole local E2 folder (I think I was just too lazy to get it to work), initially as a way of ctrl-shift-f'ing for problematic code, but it came in handy when my server's backups proved more resilient than my mate's, and I was able to (shock him) by restoring his collection...

player:SetAnimation(PLAYER_ATTACK1)
elseif (chip.alwaysAllow and chip.alwaysAllow[player]) or not IsValid(chip.player) then -- If the tooling player is in the chip's always allow table, or the chip has no valid owner meaning we can't send a request, do a CanTool check
if hook.Run("CanTool", player, WireLib.dummytrace(chip), "wire_expression2") then
self:Download(player, chip)
player:SetAnimation(PLAYER_ATTACK1)
end
elseif CheckBypass(player) then
if hook.Run("CanTool", player, WireLib.dummytrace(chip), "wire_expression2") then
-- Warn the chip's owner their E2 was just taken via the admin bypass
BetterChatPrint(chip.player, "Warning, the server admin '"..player:Nick().."' just accessed your chip '"..chip.name.."', as the view request admin bypass is enabled!")
self:Download(player, chip)
player:SetAnimation(PLAYER_ATTACK1)
end
else
RequestView(chip, player)
player:SetAnimation(PLAYER_ATTACK1)
end
else
net.Start("WireExpression2_OpenEditor") net.Send(player)
end
end
end
net.Receive("WireExpression2_AnswerRequest", function(len, plr)
local accept, initiator, chip = net.ReadUInt(8), net.ReadEntity(), net.ReadEntity()

-- Check that this message is for a valid view request
if ValidateRequest(initiator, chip) then
-- Check that the sending player actually owns the chip they're allowing access to
if chip.player ~= plr then return end

if accept ~= 0 then
WireLib.Expression2Download(initiator, chip, nil, true)
BetterChatPrint(initiator, "Your request to view "..plr:Nick().."'s chip, '"..viewRequests[initiator][chip].name.."', was accepted!")

-- If the player chose "Always Allow", then mark the initiator as always being able to access this entity on the chip
if accept == 2 then
if not chip.alwaysAllow then chip.alwaysAllow = {} end
chip.alwaysAllow[initiator] = true
end
else
BetterChatPrint(initiator, "Your request to view "..plr:Nick().."'s chip, '"..viewRequests[initiator][chip].name.."', was declined")
end
viewRequests[initiator][chip] = nil
end
end)

function TOOL:Upload(ent)
WireLib.Expression2Upload( self:GetOwner(), ent )
end
Expand Down Expand Up @@ -117,11 +268,6 @@ if SERVER then
error("Invalid player entity (wtf??). This should never happen. " .. tostring(ply), 0)
end

if not hook.Run( "CanTool", ply, WireLib.dummytrace(targetEnt), "wire_expression2") then
WireLib.AddNotify(ply, "You're not allowed to download from this Expression (ent index: " .. targetEnt:EntIndex() .. ").", NOTIFY_ERROR, 7, NOTIFYSOUND_DRIP3)
return
end

local main, includes = targetEnt:GetCode()
if not includes or not next(includes) then -- There are no includes
local datastr = WireLib.von.serialize({ { targetEnt.name, main } })
Expand Down Expand Up @@ -314,16 +460,22 @@ if SERVER then
E2 = Entity(E2)
if canhas(player) then return end
if not IsValid(E2) or E2:GetClass() ~= "gmod_wire_expression2" then return end
if hook.Run( "CanTool", player, WireLib.dummytrace( E2 ), "wire_expression2", "request code" ) then

-- Same check as tool code
if E2.player == player then
WireLib.Expression2Download(player, E2)
WireLib.AddNotify(player, "Downloading code...", NOTIFY_GENERIC, 5, math.random(1, 4))
player:PrintMessage(HUD_PRINTCONSOLE, "Downloading code...")
if E2.player ~= player then
WireLib.AddNotify(E2.player, player:Nick() .. " is reading your E2 '" .. E2.name .. "' using remote updater.", NOTIFY_GENERIC, 5, math.random(1, 4))
E2.player:PrintMessage(HUD_PRINTCONSOLE, player:Nick() .. " is reading your E2 '" .. E2.name .. "' using remote updater.")
elseif (E2.alwaysAllow and E2.alwaysAllow[player]) or not IsValid(E2.player) then
if hook.Run("CanTool", player, WireLib.dummytrace(E2), "wire_expression2") then
WireLib.Expression2Download(player, E2)
end
elseif CheckBypass(player) then
if hook.Run("CanTool", player, WireLib.dummytrace(E2), "wire_expression2") then
-- Warn the chip's owner their E2 was just taken via the admin bypass
BetterChatPrint(E2.player, "Warning, the server admin '"..player:Nick().."' just accessed your chip '"..E2.name.."', as the view request admin bypass is enabled!")
WireLib.Expression2Download(player, E2)
end
else
WireLib.ClientError("You do not have permission to read this E2.", player)
RequestView(E2, player)
end
end)

Expand Down