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

Permission management not functioning correctly #384

Open
MeteorTheLizard opened this issue May 7, 2023 · 4 comments
Open

Permission management not functioning correctly #384

MeteorTheLizard opened this issue May 7, 2023 · 4 comments

Comments

@MeteorTheLizard
Copy link

The permission management for roles as well as categories and textChannels / voiceChannels does not function correctly. After a fair bit of research with the help of another discord user, it appears to be an issue with how the permission enums as well as everything further down the line is handled.

return 2^n

Should be return 2ULL^n

Changing it to this however causes a problem here as cdata is not recognized by the Resolver.

function Resolver.permission(obj)

And when this function is expanded to support cdata, there is still the problem that cdata cannot be a table key.

Assuming the data type is preserved for permissions, changing the flag function in the enums.lua causes problems in other areas as the gatewayIntents starts to throw errors as well when changing it to return 2ULL^n - This I haven't looked into much further.

--

Stuff that I tried to do that lead to discovering the issue:

local tBotclass = require("../botinfo")

local enums = tBotclass.tDiscordia.enums.permission

local obj_Guild = message.guild

local obj_Role_Everyone

for _,v in pairs(obj_Guild.roles) do
	if v.name == "@everyone" then
		obj_Role_Everyone = v

		break
	end
end

local obj_Category = obj_Guild:createCategory("test-category-ignore")

local obj_Permissions = obj_Category:getPermissionOverwriteFor(obj_Role_Everyone)

obj_Permissions:denyAllPermissions() -- @everyone has no rights here!

Do note that I removed all error handling here to make this code easier to understand.

The code above results in this:

image

denyAllPermissions is clearly not working correctly.

When I attempt to deny specific permissions like so:

obj_Permissions:denyPermissions(enums.readMessages,enums.changeNickname,enums.sendMessages,enums.embedLinks,enums.attachFiles,enums.addReactions,enums.useExternalEmojis,enums.readMessageHistory,enums.useSlashCommands)

This happens:

image2

A lot of permissions are being denied that I did not choose, this also happens when I use allowPermissions instead, but it allows all of these permissions instead of denying them (duh)

--

I don't know the full extent of the issue, I'm not too familiar with how Discordia works internally so this is unfortunately all the information I can provide about this issue.

One thing that I can safely say though is that ULLs (somehow) work perfectly fine on my pi which is running a 32Bit system. (0ULL - 1) returns 18446744073709551615 which is precisely the 64bit limit for long integers.

@truemedian
Copy link
Contributor

truemedian commented May 7, 2023

flag is never called with a parameter greater than 52. Numbers in Luajit (what Luvit uses) are always double precision floating point (which can hold any integer smaller than 2^53), which means that no precision is being lost here. Changing 2^n to 2ULL^n accomplishes nothing until discord defines a bitflag with bit 52+ set.

@MeteorTheLizard
Copy link
Author

I have tested the accuracy of numbers on both my X64 and armv7l system, they are indeed perfectly fine, even as just double precision floating point numbers. Which is quite surprising to me.

I changed the permission enums.lua table to include missing permissions.

enums.permission = enum {

enums.permission = enum {
	-- Cut existing ones out for clarity
	useExternalstickers = flag(37),
	sendMessagesThreads = flag(38),
	useActivities       = flag(39),
	moderateMembers     = flag(40),
	creatorAnalytics    = flag(41),
	useSoundboard       = flag(42),
	--unused              = flag(43),
	--unused              = flag(44),
	--unused              = flag(45),
	sendVoiceMessages   = flag(46)
}

This mostly resolved the problem on my X64 Windows machine. Adding the missing flags caused "denyAllPermissions" to remove almost all permissions from the newly created category with the exception of "Use External Sounds" and "Create Events". If I understand the function right, this is because of these two permissions not being in the enums table, but they're also not listed in the Developer Portal. https://discord.com/developers/docs/topics/permissions#permissions-bitwise-permission-flags - So I can't just add them to test this.

Allowing individual permissions through "allowPermissions" appears to be working correctly now. I was able to set every single permission that is currently supported individually, mixed, as well as all at once using this function.

However, the function "denyAllPermissions" does NOT work properly on my armv7l system and "allowPermissions" does not work either. Considering the numbers are correct before processing (in the enums table), I am unsure where this whole thing breaks down.

Another thing to add is that; Since it only started to work properly on my X64 machine after I added the missing enums to the table, it should be considered that, once discord adds new permissions, things will break again and cause a lot of problems! Since the bots will still be doing their things while being completely broken! This can lead to users gaining permissions that they shouldn't have! Which is always a bad thing! Especially if they suddenly end up being Administrator.

Code that I used to test this excluding error handling:

local tBotclass = require("../botinfo")

local enums = tBotclass.tDiscordia.enums.permission


local obj_Guild = message.guild

local obj_Role_Everyone

for _,v in pairs(obj_Guild.roles) do
	if v.name == "@everyone" then
		obj_Role_Everyone = v

		break
	end
end

local obj_Category = obj_Guild:createCategory("test-category-ignore")

local obj_Permissions = obj_Category:getPermissionOverwriteFor(obj_Role_Everyone)

obj_Permissions:denyPermissions(enums.readMessages)

obj_Permissions:denyAllPermissions()

obj_Permissions:allowPermissions(enums.readMessages,enums.changeNickname,enums.sendMessages,enums.embedLinks,enums.attachFiles,enums.addReactions,enums.useExternalEmojis,enums.readMessageHistory,enums.useExternalstickers,enums.useSlashCommands)

Results:

X64 - Windows

x64

armv7l - Raspberry PI OS x86

armv7l

Considering that "allowPermissions" also denies some permissions here, some numbers clearly end up being messed up.
I'm unsure how this works so I cannot debug it further than this.

--

Considering there are two new permissions that will most likely be added soon "Use External Sounds" and "Create Events", it might be a good idea to properly future proof this while also making it compatible with x86 preferably. Enums missing from the enum table breaking everything is quite something.

@SinisterRectus
Copy link
Owner

I'm not sure exactly what's going on here. Can you dump the contents of your enum table? Just try p(discordia.enums.permission).

@MeteorTheLizard
Copy link
Author

MeteorTheLizard commented May 8, 2023

A bit of a mess since I have 2 platforms with 2 variants, so I'm just dumping all options here.

Modified table:
X64 Windows

{ connect = 1048576, createInstantInvite = 1, kickMembers = 2, banMembers = 4, 
  manageChannels = 16, manageGuild = 32, addReactions = 64, viewAuditLog = 128, 
  prioritySpeaker = 256, readMessages = 1024, sendMessages = 2048, 
  sendTextToSpeech = 4096, manageMessages = 8192, embedLinks = 16384, 
  attachFiles = 32768, readMessageHistory = 65536, mentionEveryone = 131072, 
  useExternalEmojis = 262144, viewGuildInsights = 524288, speak = 2097152, 
  muteMembers = 4194304, deafenMembers = 8388608, moveMembers = 16777216, 
  useVoiceActivity = 33554432, changeNickname = 67108864, 
  manageNicknames = 134217728, manageRoles = 268435456, 
  manageWebhooks = 536870912, manageEmojis = 1073741824, 
  useSlashCommands = 2147483648, requestToSpeak = 4294967296, 
  manageEvents = 8589934592, manageThreads = 17179869184, 
  usePublicThreads = 34359738368, usePrivateThreads = 68719476736, 
  useExternalstickers = 137438953472, sendMessagesThreads = 274877906944, 
  useActivities = 549755813888, moderateMembers = 1099511627776, 
  creatorAnalytics = 2199023255552, useSoundboard = 4398046511104, 
  sendVoiceMessages = 70368744177664, administrator = 8, stream = 512 }

Unmodified table:
X64 Windows

{ createInstantInvite = 1, kickMembers = 2, banMembers = 4, 
  manageChannels = 16, manageGuild = 32, addReactions = 64, viewAuditLog = 128, 
  prioritySpeaker = 256, readMessages = 1024, sendMessages = 2048, 
  sendTextToSpeech = 4096, manageMessages = 8192, embedLinks = 16384, 
  attachFiles = 32768, readMessageHistory = 65536, mentionEveryone = 131072, 
  useExternalEmojis = 262144, viewGuildInsights = 524288, speak = 2097152, 
  muteMembers = 4194304, deafenMembers = 8388608, moveMembers = 16777216, 
  useVoiceActivity = 33554432, changeNickname = 67108864, 
  manageNicknames = 134217728, manageRoles = 268435456, 
  manageWebhooks = 536870912, administrator = 8, useSlashCommands = 2147483648, 
  requestToSpeak = 4294967296, manageEvents = 8589934592, 
  manageThreads = 17179869184, usePublicThreads = 34359738368, 
  usePrivateThreads = 68719476736, connect = 1048576, stream = 512, 
  manageEmojis = 1073741824 }

--

Modified table:
armv7l x86

{ manageWebhooks = 536870912, manageEmojis = 1073741824,
useSlashCommands = 2147483648, requestToSpeak = 4294967296,
manageEvents = 8589934592, manageThreads = 17179869184,
usePublicThreads = 34359738368, usePrivateThreads = 68719476736,
useExternalstickers = 137438953472, sendMessagesThreads = 274877906944,
useActivities = 549755813888, moderateMembers = 1099511627776,
createInstantInvite = 1, kickMembers = 2, banMembers = 4,
manageChannels = 16, manageGuild = 32, addReactions = 64, viewAuditLog = 128,
prioritySpeaker = 256, connect = 1048576, sendMessages = 2048,
sendTextToSpeech = 4096, manageMessages = 8192, embedLinks = 16384,
attachFiles = 32768, readMessageHistory = 65536, mentionEveryone = 131072,
useExternalEmojis = 262144, viewGuildInsights = 524288, speak = 2097152,
muteMembers = 4194304, deafenMembers = 8388608, administrator = 8,
useVoiceActivity = 33554432, changeNickname = 67108864,
manageNicknames = 134217728, manageRoles = 268435456,
sendVoiceMessages = 70368744177664, useSoundboard = 4398046511104,
creatorAnalytics = 2199023255552, stream = 512, readMessages = 1024,
moveMembers = 16777216 }

Unmodified table:
armv7l x86

{ changeNickname = 67108864, manageNicknames = 134217728,
manageRoles = 268435456, manageWebhooks = 536870912,
manageEmojis = 1073741824, useSlashCommands = 2147483648,
requestToSpeak = 4294967296, manageEvents = 8589934592,
manageThreads = 17179869184, usePublicThreads = 34359738368,
usePrivateThreads = 68719476736, createInstantInvite = 1, kickMembers = 2,
banMembers = 4, manageChannels = 16, manageGuild = 32, addReactions = 64,
viewAuditLog = 128, prioritySpeaker = 256, stream = 512, sendMessages = 2048,
sendTextToSpeech = 4096, manageMessages = 8192, embedLinks = 16384,
attachFiles = 32768, readMessageHistory = 65536, mentionEveryone = 131072,
useExternalEmojis = 262144, connect = 1048576, administrator = 8,
readMessages = 1024, viewGuildInsights = 524288, speak = 2097152,
muteMembers = 4194304, deafenMembers = 8388608, moveMembers = 16777216,
useVoiceActivity = 33554432 }

Edit: Sorting these and then running them through a diff checker revealed that both the modified and unmodified tables are exactly the same on both platforms.

truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
Co-Authored-By: Meteor <[email protected]>
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
Co-authored-by: Meteor <[email protected]>
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
truemedian added a commit to truemedian/Discordia that referenced this issue May 11, 2023
works around a bug in luajit that seems to happen when passing a
number to `bit.bor` that is larger than 2^31-1

Additionally adds a few missing enumerations so that enableAll and
disableAll work as expected.

Fixes SinisterRectus#384
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

Successfully merging a pull request may close this issue.

3 participants