Yellow-Dog-Man / Resonite-Issues

Issue repository for Resonite.
https://resonite.com
134 stars 2 forks source link

Values on SimpleAvatarProtection component should not be able to be modified by anyone that isn't the user assigned to the comp #2759

Open LeCloutPanda opened 1 month ago

LeCloutPanda commented 1 month ago

Describe the bug?

By default if anyone but the assigned user tries messing with values on the component it self destructs but it appears ReassignUserOnResonitePackageImport doesn't have the same functionality.

To Reproduce

Just press the ReassignUserOnResonitePackageImport boolean when not the assigned user

Expected behavior

It to not change/nuke the component/avatar/object when interacted with by anyone beside the assigned user.

Screenshots

Example image

Resonite Version Number

2024.8.5.1341

What Platforms does this occur on?

Windows

What headset if any do you use?

N/A

Log Files

N/A

Additional Context

No response

Reporters

No response

shiftyscales commented 1 month ago

It to not change/nuke the component/avatar/object when interacted with by anyone beside the assigned user.

Can you describe why this is your expected behaviour, @LeCloutPanda?

Attempts at tampering with the component by a user that is not assigned to the component is what triggers that to happen.

If that box is already checked- the target user would have to import the Resonite package and the components should be assigned to them and they can then modify anything about the component they please.

By default if anyone but the assigned user tries messing with values on the component it self destructs but it appears ReassignUserOnResonitePackageImport doesn't have the same functionality.

But based on your replication steps and expected behaviour in the following lines, it sounds like it does have the same functionality?

Can you please clarify your issue?

Frooxius commented 1 month ago

This isn't really a bug, the component isn't meant to verify non-owners changing this field in particular.

We could change that behavior, but question there would be why?

The only attack I could see with this is: 1) Covertly check this field on another user's avatar 2) Convince them to export this instance as Resonite Package 3) Convince them to send the package to you 4) Import it and have it re-assigned to you

I think this scenario is so far fetched and unlikely, that it wouldn't be worth the development effort.

LeCloutPanda commented 1 month ago

The expected behavior is that if anyone beside the assigned user presses any of the input fields that the comp ignores the press/nukes it self rather then allowing the option to be changed at all by anyone, most users including my self would like it not to be modifiable by anyone but the assigned user and on top that variable is set to false

LeCloutPanda commented 1 month ago

This isn't really a bug, the component isn't meant to verify non-owners changing this field in particular.

We could change that behavior, but question there would be why?

The only attack I could see with this is:

  1. Covertly check this field on another user's avatar
  2. Convince them to export this instance as Resonite Package
  3. Convince them to send the package to you
  4. Import it and have it re-assigned to you

I think this scenario is so far fetched and unlikely, that it wouldn't be worth the development effort.

Understandable but at the very least it is inconsistent with the rest of the component, if you mess with the rest it just dies or in the case of the enabled field it doesn't do anything as do a lot of other components but is just there because inheritance so at the very least have it ignore presses/setting the value if the user who presses it isn't the assigned user.

shiftyscales commented 1 month ago

nukes it self rather then allowing the option to be changed at all by anyone

So clarifying the issue simply:

Am I understanding that correctly, @LeCloutPanda?

Frooxius commented 1 month ago

The component will self-destruct if you mess with any fields that would let you take ownership of it easily.

ReassignUserOnResonitePackageImport doesn't fall under this, so it's not protected the same way.

The question there is - why protect this field this way?