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

OBJECT_TEMP_ATTACHED rejection in oc_API for interface channel #1096

Merged
merged 4 commits into from
Sep 18, 2024

Conversation

Medea-Destiny
Copy link
Collaborator

Scottie Muircastle just pointed out to me a potential vulnerability in the object interface function. As temporary attachments become "owned" by the wearer, in theory people could use a temporary attachment that's attached as a furniture prop to send OC command with the collar wearer's auth. This was an issue previously with RLVa rejecting RLV commands from temp attachments as it's a potential vector for bypassing auth.

I've chucked in an extra line to simply reject arbitary commands from temp attachments which should resolve this. We should consider having this as a user setting or similar. I haven't had time to test this as yet but as it's a potential security flaw we need to consider this as a security patch for 8.3

@Medea-Destiny Medea-Destiny added 8.3 Needs testing This issue needs volunteers to try to duplicate the error or identify a caues labels Sep 13, 2024
@Medea-Destiny
Copy link
Collaborator Author

For ease of testing I've made an object you can attach manually and it'll use the interface channel to make you kneel. However if you rez it and click it, it'll ask permission to temp attach, and then you won't kneel. Ask me in world if you want a copy for testing.

@Medea-Destiny
Copy link
Collaborator Author

Medea-Destiny commented Sep 13, 2024

We could consider if an object is a temp attachment, adding a check for OBJECT_REZZER_KEY and allowing it to pass auth based on the rezzing object, but this will take a little consideration so I recommend targeting 8.4 for that.

This could be done with

list t=llGetObjectDetails(i,[OBJECT_TEMP_ATTACHED,OBJECT_REZZER_KEY]);
if(llList2Integer(t,0)==1) i=llList2Key(t,1);

@NikkiLacrima
Copy link
Contributor

If I understand the code right it rejects temp attachment both for wearer and for other avatars ??

@Medea-Destiny
Copy link
Collaborator Author

Right. The point here is that a temp-attached item will have the wearer as owner, and the object auth relies on the object owner for primary auth. The potential risk scenario here is that for example someone could sit on an item of furniture that temp-attaches an object, potentially using experience permissions (avsitter experiencewill do this for example). That item then inherits the wearer's auth level even though as a temp attachment it's not something the wearer necessarily has any knowledge of what it's doing.

This patch rejects anything that's temp attached, which makes sense because another avatar with auth permission on a collar could be the vector of such an exploit.

@NikkiLacrima NikkiLacrima self-requested a review September 16, 2024 19:40
Copy link
Contributor

@NikkiLacrima NikkiLacrima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@Pingout Pingout removed the Needs testing This issue needs volunteers to try to duplicate the error or identify a caues label Sep 18, 2024
@Pingout
Copy link
Collaborator

Pingout commented Sep 18, 2024

Very good. It's some what of a convoluted volnerability, but we can't leave it. Thanks for the fix.

@Pingout Pingout merged commit a7b26e9 into 8.3_Features-branch Sep 18, 2024
2 checks passed
@Pingout Pingout deleted the Medea-Destiny-temp-attach-patch branch September 18, 2024 02:09
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 this pull request may close these issues.

3 participants