Using a "group" group override doesn't apply

[bottiger1]

I've been testing sourcemod groups and found some issues with it that make it completely unusuable.

When adding a single "group" group override, it doesn't seem to work, and it causes a segfault when I run sm_dump_admcache.

sm_admins_groups admin_id,group_id,inherit_order 17,1,0

sm_groups id,flags,name,immunity_level 1,b,superadmin,0

sm_group_overrides group_id,type,name,access 1,group,testing,allow

When I change the type to a command, it works without crashing.

group_id,type,name,access 1,command,testing,allow

However, when I add another group override, it doesn't crash anymore but it gives the wrong answers in sm_dump_admcache. No matter what I change the group override to, it takes the name of the command override.

group_id,type,name,access 1,command,testing,allow 1,group,test,allow

"Groups"
{
        /* num = 1, gid = 0x0 */
        "Default"
        {
                "flags"                 ""
                "immunity"              "1"

                "Overrides"
                {
                }
        }

        /* num = 2, gid = 0x2C */
        "Full Admins"
        {
                "flags"                 "abcdefghiz"
                "immunity"              "99"

                "Overrides"
                {
                }
        }

        /* num = 3, gid = 0x5C */
        "superadmin"
        {
                "flags"                 "b"
                "immunity"              "0"

                "Overrides"
                {
                        "@testing"              "allow"
                        "testing"               "allow"
                }
        }
}

"Admins"
{
}

With #define _DEBUG in admin-sql-threaded.sp

OnDatabaseConnect(53b0491,5430499,0) ConnectLock=0 Adding group (0, b, superadmin) AddAdmGroupCmdOverride(92, testing, 1, 1) AddAdmGroupCmdOverride(92, test, 2, 1)

Running the following code in a seperate plugin with nothing in the sm_group_overrides table also results in a crash when sm_dump_admcache is run.

AddAdmGroupCmdOverride(FindAdmGroup("superadmin"), "groupoverride", Override_CommandGroup, Command_Allow);

[bottiger1]

Even easier repro:

public OnPluginStart()
{
    GroupId gid = CreateAdmGroup("testgroup");
    AddAdmGroupCmdOverride(gid, "groupoverride", Override_CommandGroup, Command_Allow);
}

[KyleSanderson]

Do you have a stack by chance? Thanks for the small test case, I just don't have any gear lying around to spin something up.

[bottiger1]

#0  0xe370ac3a in AdminCache::DumpCache(char const*) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_ze/bin/sourcemod.logic.so
[Current thread is 1 (Thread 0xf7cdc700 (LWP 4741))]
#0  0xe370ac3a in AdminCache::DumpCache(char const*) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_ze/bin/sourcemod.logic.so
#1  0xe37237e7 in ?? () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_ze/bin/sourcemod.logic.so
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

I've tried to look into the source code myself here https://github.com/alliedmodders/sourcemod/blob/1fbe5e1daaee9ba44164078fe7f59d862786e612/core/logic/AdminCache.cpp but couldn't find any issues myself before getting tired of reading the whole thing.

The fact that it doesn't work though with this RegAdminCmd("sm_test", Cmd_Test, ADMFLAG_CUSTOM6, "desc", "testgroup"); seems to indicate that there's a deep flaw with the system itself rather than just the dumpcache function though.

[asherkin]

#1519 should fix the bug with sm_dump_admcache at least.

[peace-maker]

Is there still an issue or was the buggy sm_dump_admcache output confusing you? Is there a problem with the permissions inherited from a command group override?

[bottiger1]

I did not test it recently but unless the group system relies on sm_dump_admcache to work, which I highly doubt, "group" group overrides are still broken and don't seem to work at all.

[peace-maker]

After looking into it a bit, the group commandgroup overrides aren't checked correctly in AdminCache::CheckAdminCommandAccess. It tries to lookup a commandgroup override using the command's name instead of the group the command is associated with.

https://github.com/alliedmodders/sourcemod/blob/dc9c52bfd6da64a0f25594a07f29c94f93e7c396/core/logic/AdminCache.cpp#L2052-L2057

It should look something like this after we grabbed the group of the cmd.

override = GetGroupCommandOverride(gid, cmdgroup, Override_CommandGroup, &rule); 

We'd need to lookup the command and grab the group it is in from ConCmdManager, but I don't think that's exposed through the bridge from core to logic yet.

I've tested with this:

public void OnPluginStart() {
    DumpAdminCache(AdminCache_Groups, false);
    DumpAdminCache(AdminCache_Overrides, false);

    GroupId gid = CreateAdmGroup("testgroup");
    AddAdmGroupCmdOverride(gid, "cmdsecret", Override_CommandGroup, Command_Allow);
    RegAdminCmd("sm_secret", Cmd_Secret, ADMFLAG_CUSTOM4, "Do something secret", "cmdsecret");
    
    AdminId aid = CreateAdmin();
    PrintToServer("Access to sm_secret: %d", CheckAccess(aid, "sm_secret", ADMFLAG_ROOT));
    aid.SetFlag(Admin_Custom4, true);
    PrintToServer("Access to sm_secret after SetFlag(Admin_Custom4, true): %d", CheckAccess(aid, "sm_secret", ADMFLAG_ROOT));
    aid.SetFlag(Admin_Custom4, false);
    PrintToServer("Access to sm_secret after SetFlag(Admin_Custom4, false): %d", CheckAccess(aid, "sm_secret", ADMFLAG_ROOT));

    PrintToServer("Access to cmdsecret before adding to testgroup: %d", CheckAccess(aid, "cmdsecret", ADMFLAG_ROOT));
    PrintToServer("InheritGroup(%d) = %d", gid, aid.InheritGroup(gid));
    PrintToServer("Access to sm_secret after adding to testgroup: %d !!!!!!!!!!", CheckAccess(aid, "sm_secret", ADMFLAG_ROOT)); // Should have access.
    PrintToServer("Access to cmdsecret after adding to testgroup: %d", CheckAccess(aid, "cmdsecret", ADMFLAG_ROOT));
}

public Action Cmd_Secret(int client, int args) {
    PrintToServer("Cmd_Secret");
    return Plugin_Handled;
}

Access to sm_secret: 0
Access to sm_secret after SetFlag(Admin_Custom4, true): 1
Access to sm_secret after SetFlag(Admin_Custom4, false): 0
Access to cmdsecret before adding to testgroup: 0
InheritGroup(0) = 1
Access to sm_secret after adding to testgroup: 0 !!!!!!!!!!
Access to cmdsecret after adding to testgroup: 1

Now we need to decide if using CheckAccess on a command group should be valid, but I think we'll have to keep allowing that since it's been too long.