Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Request] Creating a list of cvars that are prohibited from sending values to Query Client CVar #2581

Closed
Splatt581 opened this issue Jul 11, 2019 · 13 comments
Assignees

Comments

@Splatt581
Copy link

I propose to create a list of cvars that will not send values ​​to server requests svc_sendcvarvalue and svc_sendcvarvalue2 - in response, game client can send a Bad CVAR request string.

Such a list may include, for example, cvars rcon_address, rcon_port and rcon_password. They must be private, because the remote control mechanism of the game server by the game client implies that the player does not have to be on the controlled server - using the cvars rcon_address and rcon_port, player can specify the data where rcon commands will be sent. An attacker can take advantage of this and, having lured the victim-admin to his server, obtain the values ​​of the cvars rcon_address, rcon_port and rcon_password via svc_sendcvarvalue or svc_sendcvarvalue2.

@SamVanheer
Copy link

I've already reported this on HackerOne with those cvars listed.

@mikela-valve you should add this issue to the private one.

@mikela-valve mikela-valve self-assigned this Jul 11, 2019
@mikela-valve mikela-valve added this to the Next Release milestone Jul 11, 2019
@afwn90cj93201nixr2e1re
Copy link

@mikela-valve can you also make setinfo buffer lil more than now? Or make some cmd for auto clearing?
For reset to default, coz some server's making some stupid records, then we get info buffer exceeded blah blah blah, then we must close hame, clear config and start game again.

@SamVanheer
Copy link

Changing the info buffer size will break this API:

halflife/common/com_model.h

Lines 315 to 349 in c7240b9

#define MAX_INFO_STRING 256
#define MAX_SCOREBOARDNAME 32
typedef struct player_info_s
{
// User id on server
int userid;
// User info string
char userinfo[ MAX_INFO_STRING ];
// Name
char name[ MAX_SCOREBOARDNAME ];
// Spectator or not, unused
int spectator;
int ping;
int packet_loss;
// skin information
char model[MAX_QPATH];
int topcolor;
int bottomcolor;
// last frame rendered
int renderframe;
// Gait frame estimation
int gaitsequence;
float gaitframe;
float gaityaw;
vec3_t prevgaitorigin;
customization_t customdata;
} player_info_t;

Same for physics info buffers.

@GiovaniFerraroTrivelli
Copy link

@mikela-valve can you also make setinfo buffer lil more than now? Or make some cmd for auto clearing?
For reset to default, coz some server's making some stupid records, then we get info buffer exceeded blah blah blah, then we must close hame, clear config and start game again.

A command for reset to default userinfo values is a good idea 👍

@10th
Copy link

10th commented Jul 13, 2019

exec clear_si.cfg ;-)

@afwn90cj93201nixr2e1re
Copy link

and? This do nothing. Coz this doesn't rewrite any setinfo's. It's can just replace some of keys.

@2010kohtep
Copy link

Setinfo restorer from one of my project. It would be nice to have something like this officially, I think.

image

@mikela-valve
Copy link
Member

@2010kohtep Could you post that as a separate issue for tracking? I'm going to implement something like that but it might take a bit more discussion to finish.

@2010kohtep
Copy link

@mikela-valve Sure thing: #2589

@mikela-valve
Copy link
Member

This should be fixed now in the current beta.

@Splatt581
Copy link
Author

Fixed, I confirm. Now, in response to requests for these cvars, game client sends string CVAR is privileged. Also, the server cannot change values of these cvars via svc_stufftext in order to send password to the changed address.

I think that client command rcon should also be added to the list of privileged ones, so that the attacker could not control the victim’s server through the victim’s client using svc_stufftext.

@mikela-valve
Copy link
Member

I agree, I'll make rcon privileged as well.

@Splatt581
Copy link
Author

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants