Anaminus / rbxmk

A tool for processing Roblox files.
MIT License
109 stars 11 forks source link

Invalid reference in Model.PrimaryPart causes crash #51

Open RaspyPiStyLiS opened 2 years ago

RaspyPiStyLiS commented 2 years ago

Using the following script rbxmk produces a .rbxm file that crashes roblox studio. Through some debugging I've determined that it's related to the PrimaryPart property on Model1

PoC Script:

local Datamodel = fs.read("Bruh.rbxm", "rbxm") --A roblox generated rbxm file containing a "Model" Instance
Datamodel[sym.RawDesc] = rbxmk.globalDesc --Use roblox's default descriptors.

local Model0 = Instance.new("Model") --Create a new model in rbxmk
local Model1 = Datamodel:GetChildren()[1]
Model1.Parent = Model0
Model0.Parent = Datamodel

fs.write("PoC.rbxm", Datamodel, "rbxm") --This written file will crash studio on load

Command line in use: rbxmk run -desc-latest bruh.lua

Reproduction Instructions:

  1. Go into roblox studio and create a model instance
  2. Save it as a file in the roblox binary format
  3. Name it to bruh.rbxm in the working directory or alter the script above to load it in
  4. Take the outputted PoC.rbxm file and load it into roblox studio
  5. It will instantly close due to a crash.

Interestingly enough doing this works around the issue:

local Datamodel = fs.read("Bruh.rbxm", "rbxm") --A roblox generated rbxm file containing a "Model" Instance
Datamodel[sym.RawDesc] = rbxmk.globalDesc --Use roblox's default descriptors.

local wtf = Instance.new("BasePart") --This ends up not getting deserialized, but it fixes the issue?
wtf.Parent = Datamodel

local Model0 = Instance.new("Model") --Create a new model in rbxmk
local Model1 = Datamodel:GetChildren()[1]
Model1.Parent = Model0
Model0.Parent = Datamodel

fs.write("PoC.rbxm", Datamodel, "rbxm") --This written file will crash studio on load
Anaminus commented 2 years ago

So here's the rundown: the script operates on two instances with different sets of properties. The studio-sourced Model contains all Model properties, while the rbxmk-sourced Model contains none of them (by design). However, the RBXM format requires that all instances of the same class have the same property set. Because the two models have different sets, the encoder resolves this by filling the missing properties with zero-values. In particular, missing properties of the Reference type are filled in with 0, which is a reference to an existing instance. In this case, that is the rbxmk-sourced Model. The PrimaryPart property (a Reference) of the rbxmk-sourced Model is filled in this way, which means that the PrimaryPart of the Model is being set to the Model itself. Apparently Studio does not like that the PrimaryPart is a non-BasePart, so it crashes.

The reason why the second script does not crash is because the PrimaryPart now points to the "BasePart" instance. It seems that Studio silently discards such invalid classes, so the reference resolves to nothing, which is a valid state.

The problem to be resolved in rbxmk is that missing properties need to be handled better. A possible solution may be to prefill value arrays with the identity values for each type. It may also be possible to encode instances with diverging property sets in separate class chunks, though this use of the binary format may not be supported by Roblox.

I'll also make a bug report on the DevForum regarding the crash.