Object leaking via Initial Snapshots in s&box

Introduction

For anyone interested in s&box's networking, networked visibility usually looks pretty solid on paper. You implement INetworkVisible on a component, turn off AlwaysTransmit, and the engine calls IsVisibleToConnection per-connection before it bothers sending Sync or Transform updates. If the callback says no, that client simply doesn't get updates for the object. Clean, per-connection culling, exactly what you want for things like fog-of-war or hidden enemies.

The problem was this only applied to the ongoing update stream. It never applied to the very first packet a client receives when it joins the game: the initial snapshot.

The Bug

So someone had pointed this issue to me where the initial snapshot builder was completely bypassing visibility rules.

Rather than checking if an entity should actually be seen by a connecting client, the server just serializes the current state of every networked object and hands it to the new client in one go. But the snapshot builder wasn't consulting INetworkVisible at all. It walked every networked object regardless of AlwaysTransmit or visibility state, and sent its current WorldPosition and [Sync] values straight to the joining client.

The code itself looked very simple, the loop just blasted everything out:

public void WriteInitialSnapshot( NetworkConnection connection, NetWrite buffer )
{
    foreach ( var netObject in NetworkedObjects )
    {
        buffer.Write( netObject.Guid );
        buffer.Write( netObject.Transform.WorldPosition );
        netObject.NetworkTable.WriteAll( buffer );
    }
}

Somehow though the client was getting a full snapshot of every networked object's current state right when they connected, including ones they should never have been able to see. Then from the very next tick onward, visibility culling kicks in correctly and the client stops receiving updates for anything IsVisibleToConnection says no to.

That's a single frame of unrestricted access followed by correctly enforced restrictions forever after. It sounds minor until you think about what current WorldPosition of every networked entity actually means in a PvP context. A client can join, grab one packet's worth of positions for every player on the server regardless of range or visibility rules, and use that as a seed for wallhacks or ESP tooling. The ongoing culling being airtight doesn't help, because the damage is already done at the moment of connection.

The Fix

The PR for this fix is here

Changing the code means the initial snapshot path now uses the same visibility check as the ongoing update path. When the server builds the snapshot for a newly connecting client, it checks IsVisibleToConnection for every object that has AlwaysTransmit turned off, and simply omits any object the callback rejects.

The code looks like this now:

public void WriteInitialSnapshot( NetworkConnection connection, NetWrite buffer )
{
    foreach ( var netObject in NetworkedObjects )
    {
        if ( !netObject.AlwaysTransmit && netObject is INetworkVisible visibleObject )
        {
            if ( !visibleObject.IsVisibleToConnection( connection ) )
            {
                continue; 
            }
        }

        buffer.Write( netObject.Guid );
        buffer.Write( netObject.Transform.WorldPosition );
        netObject.NetworkTable.WriteAll( buffer );

        connection.TrackedObjects.Add( netObject.Guid );
    }
}

Now the joining client no longer gets a free look at objects it was never meant to see. If the client's connection later becomes a valid recipient for that object, the existing visibility-change path already handles pushing it to them at that point, so nothing new had to be built there.

Currently I am working on trying to introduce dormant objects! This significantly reduces the amount of data that needs to be sent to clients, and also reduces the amount of work the server has to do.