Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0008903opensim[GRID] Grid Servicepublic2021-06-27 19:242021-11-03 09:02
Reportertampa 
Assigned To 
PriorityurgentSeveritymajorReproducibilityalways
Statuspatch includedResolutionopen 
PlatformOperating SystemOperating System Version
Product Version 
Target VersionFixed in Version 
Summary0008903: Entry of bad inventory data possible
DescriptionOut of curiosity for alternative ways to access our lovely virtual worlds I tried a few different viewers departing from the recommended, built for OpenSim, stack of viewers to the supposed still "compatible" SL-first viewers.

Unfortunately in doing so I discovered that some viewers handle inventory data a whole lot differently, which then subsequently breaks inventory blocking logins from any viewer.

The viewer I last tried and thus have to assume is at least one of the problematic ones was Kirsten viewer. It set the inventory folders UUID to uppercase rather than lowercase and inserted the folder UUID of the "My Inventory" root folder into the parent UUID of the same folder. Normally this would be a null key.

Result: Login failed message when trying to login even when using a viewer like Firestorm. The inventory folders are not reset/fixed either requiring manual edits in the database. Currently Firestorm also does not know what to make of this data so there is no specific failure message that inventory data is bad.

An immediate fix to this would be making sure the parent UUID of the "My Inventory" folder cannot be set to the folder UUID or in fact any folder be its own parent... that just doesn't make sense anyways.

Also forcing all the UUIDs for folders, parent included, to be at all times lowercase, thus also preventing entry of the same folder structures simply using uppercase UUIDs.
Steps To ReproduceLogin with Kirsten or supposedly and SL-first viewer.

Observe database inventory folders table for changes.
Additional InformationI have the first part of this done as patch, looking into the second part forcing all UUIDs to lowercase is a bit more difficult as ideally this would be done by converting them to lowercase before anything further is processed, but not sure where it makes most sense to insert that.

There really needs to be some care given to data entry, OpenSim allows far too much bad data to get in with potentially disastrous effects.
TagsNo tags attached.
Git Revision or version number
Run ModeStandalone (1 Region) , Standalone (Multiple Regions) , Grid (1 Region per Sim) , Grid (Multiple Regions per Sim)
Physics EngineBulletSim
Script EngineXEngine
EnvironmentMono / Linux64
Mono Version6.x
ViewerKirsten, Firestorm
Attached Filespatch file icon 0082-Prevent-bad-inventory-data.patch [^] (18,304 bytes) 2021-11-02 17:43 [Show Content]
patch file icon 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch [^] (1,234 bytes) 2021-11-03 09:02 [Show Content]

- Relationships

-  Notes
(0038210)
tampa (reporter)
2021-11-02 17:29

I added a patch that is a bit overkill on this, it specifically forces lowercase UUIDs for the inventory folders and other relevant UUIDs.

Also trying to bail on setting folderID and parentFolderID to the same, which prevents login as that expect null key for most parent "My Inventory" folder.

I added all that stuff to doubly make sure and to show where bad data entry might be possible.

Also had to fix the test for inventory, because CreatorId is a string and the PropertyScrambler thus creates a random string in Uppercase and not a lowercase UUID. Not sure how best to fix that long term.

At least the bailout for the parentID of inventory folders should be merged since that is breaking logins, so even if a user just attempts to use another viewer they might break their inventory permanently and without access to inventory database this cannot be rectified either. Even a "good" viewer will not fix that back to the proper null key.

-----------

I did more tests with Kirsten viewer and got the avatar logged in... well kinda, it doesn't load because baking fails and some structures it expects differ too much for it continue, but OpenSim does see the avatar and it gets to the region. Other avatars can see it fine.

Kirsten seems to set all inventory folders as Uppercase and really badly wants "My Inventory" to have the its parent folder be its own ID. Not sure what the reasoning for that is, but given supposedly it logs to SL this might be their inventory spec.

Though the Uppercase UUID stuff is just stupid.

Not sure if parent == folderid should be adjusted for. On the inventory fetch for login response it checks for null key, guess this could be adjusted to also allow for this spec. I don't have SL database obviously so no idea how that looks on this. Seems silly.
(0038211)
tampa (reporter)
2021-11-03 07:35

Recent changes on master might help prevent this, that is nice to see.

Still have to figure out how to fix the property scrambler, but I think I have an idea on how to solve that.

Meantime would someone remove the "how to get SL viewer connected" from the wiki before more people brick their inventories :)
(0038212)
tampa (reporter)
2021-11-03 08:47

I fully admit this is a horrid way of doing it, but frankly for these tests it doesn't really matter much anyways. Trying to parse the string as UUID, Guid, heck even regex match to format was just not working out, can't say why, copied the code over from various other places such checks are performed, but it just doesn't want to understand it.

At least now the damn thing figures out the input contains hyphens thus is most likely a UUID, not that it matters much if you name something as a UUID anyways, but this way that dreaded CreatorId field will properly populate and the test actually gets a lowercase UUID instead of a uppercase string.

Granted this is minor and petty, but given how messy these tests run surprised some of them work at all and really are not a good guarantee things work properly if they work with bad data in the first place... I'll stop ranting now.

- Issue History
Date Modified Username Field Change
2021-06-27 19:24 tampa New Issue
2021-06-27 19:24 tampa File Added: 0082-Prevent-inventory-folders-name-parent.patch
2021-11-02 17:19 tampa File Added: 0082-Prevent-bad-inventory-data.patch
2021-11-02 17:20 tampa File Deleted: 0082-Prevent-inventory-folders-name-parent.patch
2021-11-02 17:29 tampa Note Added: 0038210
2021-11-02 17:29 tampa Status new => patch included
2021-11-02 17:43 tampa File Deleted: 0082-Prevent-bad-inventory-data.patch
2021-11-02 17:43 tampa File Added: 0082-Prevent-bad-inventory-data.patch
2021-11-03 07:35 tampa Note Added: 0038211
2021-11-03 08:43 tampa File Added: 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch
2021-11-03 08:47 tampa Note Added: 0038212
2021-11-03 08:49 tampa File Deleted: 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch
2021-11-03 08:50 tampa File Added: 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch
2021-11-03 09:02 tampa File Deleted: 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch
2021-11-03 09:02 tampa File Added: 0001-Teach-PropertyScrambler-UUIs-stupid-way.patch


Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker