Skip to content

Commit

Permalink
engine: fix potential UB in netadr_t
Browse files Browse the repository at this point in the history
  • Loading branch information
a1batross committed Feb 6, 2025
1 parent d4c34ab commit add02dc
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 78 deletions.
61 changes: 51 additions & 10 deletions common/netadr.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,17 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

#define PORT_ANY -1

typedef enum {NA_LOOPBACK = 1, NA_BROADCAST, NA_IP, NA_IPX, NA_BROADCAST_IPX, NA_IP6, NA_MULTICAST_IP6} netadrtype_t;
typedef enum netadrtype_e
{
NA_UNDEFINED = 0,
NA_LOOPBACK,
NA_BROADCAST,
NA_IP,
NA_IPX,
NA_BROADCAST_IPX,
NA_IP6,
NA_MULTICAST_IP6
} netadrtype_t;

/*
Original Quake-2 structure:
Expand All @@ -46,29 +56,60 @@ typedef struct
#pragma pack( push, 1 )
typedef struct netadr_s
{
// the reason we do this evil thing, is that when this struct contains IPv6
// address the `type` is 2-byte wide, but when it doesn't `type` must 4-byte
// wide _and_ ip6_0 must be zeroed, to keep it binary compatible.
#if XASH_LITTLE_ENDIAN
uint16_t type;
uint8_t ip6_0[2];
#elif XASH_BIG_ENDIAN
uint8_t ip6_0[2];
uint16_t type;
#else
#error
#endif

union
{
// IPv6 struct
uint8_t ip6_1[14];
struct
{
uint16_t type6;
uint8_t ip6[16];
};
struct
{
uint32_t type; // must be netadrtype_t but will break with short enums
union
{
uint8_t ip[4];
uint32_t ip4; // for easier conversions
uint8_t ip[4];
uint32_t ip4; // for easier conversions
};
uint8_t ipx[10];
};
};
uint16_t port;
uint16_t port;
} netadr_t;
#pragma pack( pop )

static inline netadrtype_t NET_NetadrType( const netadr_t *a )
{
if( a->type == NA_IP6 || a->type == NA_MULTICAST_IP6 )
return a->type;

if( a->ip6_0[0] || a->ip6_0[1] )
return NA_UNDEFINED;

return a->type;
}

static inline void NET_NetadrSetType( netadr_t *a, netadrtype_t type )
{
if( type == NA_IP6 || type == NA_MULTICAST_IP6 )
{
a->type = type;
return;
}

a->ip6_0[0] = a->ip6_0[1] = 0;
a->type = type;
}

STATIC_CHECK_SIZEOF( netadr_t, 20, 20 );

#endif // NET_ADR_H
2 changes: 1 addition & 1 deletion engine/client/cl_game.c
Original file line number Diff line number Diff line change
Expand Up @@ -3427,7 +3427,7 @@ static void GAME_EXPORT NetAPI_SendRequest( int context, int request, int flags,
return;
}

if( remote_address->type == NA_IPX || remote_address->type == NA_BROADCAST_IPX )
if( NET_NetadrType( remote_address ) == NA_IPX || NET_NetadrType( remote_address ) == NA_BROADCAST_IPX )
return; // IPX no longer support

if( request == NETAPI_REQUEST_SERVERLIST )
Expand Down
29 changes: 15 additions & 14 deletions engine/client/cl_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,7 @@ static void CL_SendConnectPacket( connprotocol_t proto, int challenge )
const char *key = ID_GetMD5();
netadr_t adr = { 0 };
int input_devices;
netadrtype_t adrtype;

protinfo[0] = 0;

Expand All @@ -1073,14 +1074,16 @@ static void CL_SendConnectPacket( connprotocol_t proto, int challenge )
return;
}

adrtype = NET_NetadrType( &adr );

if( adr.port == 0 ) adr.port = MSG_BigShort( PORT_SERVER );

input_devices = IN_CollectInputDevices();
IN_LockInputDevices( adr.type != NA_LOOPBACK ? true : false );
IN_LockInputDevices( adrtype != NA_LOOPBACK ? true : false );

// GoldSrc doesn't need sv_cheats set to 0, it's handled by svc_goldsrc_sendextrainfo
// it also doesn't need useragent string
if( adr.type != NA_LOOPBACK && proto != PROTO_GOLDSRC )
if( adrtype != NA_LOOPBACK && proto != PROTO_GOLDSRC )
{
Cvar_SetCheatState();
Cvar_FullSet( "sv_cheats", "0", FCVAR_READ_ONLY | FCVAR_SERVER );
Expand Down Expand Up @@ -1218,7 +1221,7 @@ static void CL_CheckForResend( void )
cls.signon = 0;
cls.state = ca_connecting;
Q_strncpy( cls.servername, "localhost", sizeof( cls.servername ));
cls.serveradr.type = NA_LOOPBACK;
NET_NetadrSetType( &cls.serveradr, NA_LOOPBACK );
cls.legacymode = PROTO_CURRENT;

// we don't need a challenge on the localhost
Expand Down Expand Up @@ -1554,8 +1557,8 @@ static void CL_SendDisconnectMessage( connprotocol_t proto )
MSG_WriteString( &buf, "dropclient\n" );
else MSG_WriteString( &buf, "disconnect" );

if( !cls.netchan.remote_address.type )
cls.netchan.remote_address.type = NA_LOOPBACK;
if( NET_NetadrType( &cls.netchan.remote_address ) == NA_UNDEFINED )
NET_NetadrSetType( &cls.netchan.remote_address, NA_LOOPBACK );

// make sure message will be delivered
Netchan_TransmitBits( &cls.netchan, MSG_GetNumBitsWritten( &buf ), MSG_GetData( &buf ));
Expand Down Expand Up @@ -1724,19 +1727,17 @@ CL_LocalServers_f
*/
static void CL_LocalServers_f( void )
{
netadr_t adr;

memset( &adr, 0, sizeof( adr ));
netadr_t adr = { 0 };

Con_Printf( "Scanning for servers on the local network area...\n" );
NET_Config( true, true ); // allow remote

// send a broadcast packet
adr.type = NA_BROADCAST;
NET_NetadrSetType( &adr, NA_BROADCAST );
adr.port = MSG_BigShort( PORT_SERVER );
Netchan_OutOfBandPrint( NS_CLIENT, adr, A2A_INFO" %i", PROTOCOL_VERSION );

adr.type = NA_MULTICAST_IP6;
NET_NetadrSetType( &adr, NA_MULTICAST_IP6 );
Netchan_OutOfBandPrint( NS_CLIENT, adr, A2A_INFO" %i", PROTOCOL_VERSION );
}

Expand Down Expand Up @@ -2480,18 +2481,18 @@ static void CL_ServerList( netadr_t from, sizebuf_t *msg )
while( MSG_GetNumBitsLeft( msg ) > 8 )
{
uint8_t addr[16];
netadr_t servadr;
netadr_t servadr = { 0 };

if( from.type6 == NA_IP6 ) // IPv6 master server only sends IPv6 addresses
if( NET_NetadrType( &from ) == NA_IP6 ) // IPv6 master server only sends IPv6 addresses
{
MSG_ReadBytes( msg, addr, sizeof( addr ));
NET_IP6BytesToNetadr( &servadr, addr );
servadr.type6 = NA_IP6;
NET_NetadrSetType( &servadr, NA_IP6 );
}
else
{
MSG_ReadBytes( msg, servadr.ip, sizeof( servadr.ip )); // 4 bytes for IP
servadr.type = NA_IP;
NET_NetadrSetType( &servadr, NA_IP );
}
servadr.port = MSG_ReadShort( msg ); // 2 bytes for Port

Expand Down
Loading

0 comments on commit add02dc

Please sign in to comment.