Dirac reports: petz camels with saddlebags at ... #4661

Closed
opened 2023-06-02 03:16:05 +00:00 by yourland-report · 8 comments

Dirac reports a bug:

petz camels with saddlebags attached can be abused to effectively allow someone to have 2048 inventory slots.

Player position:

{
	x = 2150.1989746094,
	y = 20.5,
	z = 1124.1970214844
}

Player look:

{
	x = -0.56648641824722,
	y = -0.7613120675087,
	z = -0.31543159484863
}

Player information:

{
	max_rtt = 5.2950000762939,
	version_string = "5.6.1",
	formspec_version = 6,
	ip_version = 6,
	min_rtt = 0.10700000077486,
	protocol_version = 41,
	minor = 6,
	max_jitter = 5.1789999008179,
	avg_jitter = 0.0059999972581863,
	connection_uptime = 3920,
	serialization_version = 29,
	patch = 1,
	lang_code = "",
	major = 5,
	state = "Active",
	min_jitter = 0,
	avg_rtt = 0.11500000208616
}

Player meta:

{
	fields = {
		xp = "148994",
		punch_count = "24604",
		inflicted_damage = "518712",
		yl_church = "return {[\"last_heal\"] = 1685407971, [\"last_death\"] = {[\"y\"] = 3, [\"z\"] = 626, [\"x\"] = -107}, [\"last_death_portal\"] = 1685612074}",
		repellant = "0",
		["stamina:poisoned"] = "no",
		digged_nodes = "181047",
		arenalib_infobox_arenaID = "0",
		["ocean_build.last_warning"] = "1.68508e+09",
		["stamina:level"] = "10",
		["ocean_build.ocean_built"] = "1",
		["stamina:exhaustion"] = "157",
		partychat = "party",
		hud_state = "on",
		played_time = "2000816",
		["unified_inventory:bags"] = "return {\"unified_inventory:bag_large\", \"unified_inventory:bag_large\", \"unified_inventory:bag_large\", \"unified_inventory:bag_large\"}",
		["signslib:pos"] = "(2148,19,1121)",
		yl_commons_player_created = "1644856923",
		["3d_armor_inventory"] = "return {\"3d_armor:boots_crystal 1 29100\", \"3d_armor:chestplate_crystal 1 29100\", \"3d_armor:helmet_nether 1 5940\", \"shields:shield_rainbow 1 16790\", \"3d_armor:leggings_crystal 1 7114\", \"\"}",
		jointime = "1644856923",
		bitten = "0",
		yl_commons_thankyou = "22",
		yl_commons_player_joined = "1685671868",
		placed_nodes = "60442",
		died = "122",
		crafted = "62903"
	}
}

Log identifier


[MOD] yl_report log identifier = qHicGK8mlm6L45IJQGaW7e4zpgZrTKm3

Profiler save:

profile-20230602T031605.json_prettyEE

Status:

# Server: version: 5.7.0-yl-thx-tmm | game: Minetest Game | uptime: 4h 59min 8s | max lag: 0.202s | clients (15/52): 9T9, AliasAlreadyTaken, Aliza, Bailiff, Beth75, Dirac, flux, Lupercus, mahou, MysticalPlayz, Neon_, Nodes, Service, shanish2, the_chosen_one

Teleport command:

/teleport xyz 2150 21 1124

Compass command:

/give_compass Construction qHicGK8mlm6L45IJQGaW7e4zpgZrTKm3 D2691E 2150 21 1124
Dirac reports a bug: > petz camels with saddlebags attached can be abused to effectively allow someone to have 2048 inventory slots. Player position: ``` { x = 2150.1989746094, y = 20.5, z = 1124.1970214844 } ``` Player look: ``` { x = -0.56648641824722, y = -0.7613120675087, z = -0.31543159484863 } ``` Player information: ``` { max_rtt = 5.2950000762939, version_string = "5.6.1", formspec_version = 6, ip_version = 6, min_rtt = 0.10700000077486, protocol_version = 41, minor = 6, max_jitter = 5.1789999008179, avg_jitter = 0.0059999972581863, connection_uptime = 3920, serialization_version = 29, patch = 1, lang_code = "", major = 5, state = "Active", min_jitter = 0, avg_rtt = 0.11500000208616 } ``` Player meta: ``` { fields = { xp = "148994", punch_count = "24604", inflicted_damage = "518712", yl_church = "return {[\"last_heal\"] = 1685407971, [\"last_death\"] = {[\"y\"] = 3, [\"z\"] = 626, [\"x\"] = -107}, [\"last_death_portal\"] = 1685612074}", repellant = "0", ["stamina:poisoned"] = "no", digged_nodes = "181047", arenalib_infobox_arenaID = "0", ["ocean_build.last_warning"] = "1.68508e+09", ["stamina:level"] = "10", ["ocean_build.ocean_built"] = "1", ["stamina:exhaustion"] = "157", partychat = "party", hud_state = "on", played_time = "2000816", ["unified_inventory:bags"] = "return {\"unified_inventory:bag_large\", \"unified_inventory:bag_large\", \"unified_inventory:bag_large\", \"unified_inventory:bag_large\"}", ["signslib:pos"] = "(2148,19,1121)", yl_commons_player_created = "1644856923", ["3d_armor_inventory"] = "return {\"3d_armor:boots_crystal 1 29100\", \"3d_armor:chestplate_crystal 1 29100\", \"3d_armor:helmet_nether 1 5940\", \"shields:shield_rainbow 1 16790\", \"3d_armor:leggings_crystal 1 7114\", \"\"}", jointime = "1644856923", bitten = "0", yl_commons_thankyou = "22", yl_commons_player_joined = "1685671868", placed_nodes = "60442", died = "122", crafted = "62903" } } ``` Log identifier ``` [MOD] yl_report log identifier = qHicGK8mlm6L45IJQGaW7e4zpgZrTKm3 ``` Profiler save: ``` profile-20230602T031605.json_prettyEE ``` Status: ``` # Server: version: 5.7.0-yl-thx-tmm | game: Minetest Game | uptime: 4h 59min 8s | max lag: 0.202s | clients (15/52): 9T9, AliasAlreadyTaken, Aliza, Bailiff, Beth75, Dirac, flux, Lupercus, mahou, MysticalPlayz, Neon_, Nodes, Service, shanish2, the_chosen_one ``` Teleport command: ``` /teleport xyz 2150 21 1124 ``` Compass command: ``` /give_compass Construction qHicGK8mlm6L45IJQGaW7e4zpgZrTKm3 D2691E 2150 21 1124 ```
AliasAlreadyTaken was assigned by yourland-report 2023-06-02 03:16:06 +00:00
AliasAlreadyTaken added the
1. kind/bug
3. source/mod upstream
2. prio/critical
ugh/petz
labels 2023-06-02 03:28:36 +00:00

It is worse than originally described.

Exploit details:

  1. Camels can have saddlebags attached to them, which give them 16 inventory slots. 2. Each Camel with saddlebags has its own unique inventory. This inventory persists upon picking up a Camel.
  2. This means each inventory slot can be used to store 16 slots of items. When all bag slots are taken into account, this means 2048 effective inventory slots.
  3. Camels can be stored within another camel's saddlebags.
  4. This means that we can recursively store camels within camels, enabling a tedious, but totally feasible method for converting a single inventory slot into an unbounded number of inventory slots.
It is worse than originally described. Exploit details: 1. Camels can have saddlebags attached to them, which give them 16 inventory slots. 2. Each Camel with saddlebags has its own unique inventory. This inventory persists upon picking up a Camel. 3. This means each inventory slot can be used to store 16 slots of items. When all bag slots are taken into account, this means 2048 effective inventory slots. 4. **Camels can be stored within another camel's saddlebags.** 5. This means that we can recursively store camels within camels, enabling a tedious, but totally feasible method for converting a *single* inventory slot into an unbounded number of inventory slots.

Possible remedies:

  1. Disable saddlebags
  2. Disable picking up camels when saddlebags are attached
  3. Disable picking up camels when saddlebags are attached and inventory is not empty

I think any of these could be implemented cleanly as config options upstream. Probably 1 and 2 are the simplest to implement.

Additionally, I think recursive camel storage is a flat-out bug. There's no way Runsy intended that. I created an issue upstream for that bit.

Possible remedies: 1. Disable saddlebags 2. Disable picking up camels when saddlebags are attached 3. Disable picking up camels when saddlebags are attached and inventory is not empty I think any of these could be implemented cleanly as config options upstream. Probably 1 and 2 are the simplest to implement. Additionally, I think recursive camel storage is a flat-out bug. There's no way Runsy intended that. I created an [issue](https://github.com/runsy/petz/issues/186) upstream for that bit.

recursively store camels within camels

with deep enough recursion, the size of metadata will grow beyond limits (like max size of packet limit) and causing issues (lag for anybody seeing the camel) or just crashing the server.

> recursively store camels within camels with deep enough recursion, the size of metadata will grow beyond limits (like max size of packet limit) and causing issues (lag for anybody seeing the camel) or just crashing the server.
Member

this is not what i'd consider an "effective" infinite inventory glitch - it's a bit annoying to take out recursively stored camels or remember what's stored w/ each camel. but the metadata growth thing could actually cause problems, so it's probably worth fixing.

upstream issue that someone reported: https://github.com/runsy/petz/issues/186

i agree w/ alias that the best solution is probably to prevent picking up camels w/ saddlebags that have anything in them.

this is not what i'd consider an "effective" infinite inventory glitch - it's a bit annoying to take out recursively stored camels or remember what's stored w/ each camel. but the metadata growth thing could actually cause problems, so it's probably worth fixing. upstream issue that someone reported: https://github.com/runsy/petz/issues/186 i agree w/ alias that the best solution is probably to prevent picking up camels w/ saddlebags that have anything in them.
flux added the
4. step/ready to QA test
label 2023-06-04 22:50:08 +00:00
Member

upstream is supposedly fixed; you shouldn't be able to pick up camels w/ bags attached. use shears to remove bags.

upstream is supposedly fixed; you shouldn't be able to pick up camels w/ bags attached. use shears to remove bags.
AliasAlreadyTaken added this to the 1.1.120 milestone 2023-06-05 09:56:24 +00:00
Member

Making it impossible to pick them up sounds like a bit much. Just not allowing to store a camel in a camel bag would be best.

Seems like we need some marker for preventing recursive storage? Technic chests + wrench (if we'd have it) and now camels all need it.

Making it impossible to pick them up sounds like a bit much. Just not allowing to store a camel in a camel bag would be best. Seems like we need some marker for preventing recursive storage? Technic chests + wrench (if we'd have it) and now camels all need it.
Member

Seems like we need some marker for preventing recursive storage? Technic chests + wrench (if we'd have it) and now camels all need it.

i feel like some sort of general mechanism would come up against the halting problem, but more pragmatically, the means of restricting whether an item can go into an inventory is a hodge-podge between node entities, detached inventories, and player inventories, as well as various other mods which modify inventories directly. the mechanics for associating an inventory with a specific item are also not standardized.

> Seems like we need some marker for preventing recursive storage? Technic chests + wrench (if we'd have it) and now camels all need it. i feel like some sort of general mechanism would come up against the halting problem, but more pragmatically, the means of restricting whether an item can go into an inventory is a hodge-podge between node entities, detached inventories, and player inventories, as well as various other mods which modify inventories directly. the mechanics for associating an inventory with a specific item are also not standardized.
AliasAlreadyTaken added the
ugh/QA OK
label 2023-08-25 07:09:23 +00:00
flux added
5. result/fixed
and removed
4. step/ready to QA test
labels 2023-11-16 22:54:16 +00:00
AliasAlreadyTaken was unassigned by flux 2023-11-16 22:54:18 +00:00
Member

this is live ("You cannot capture animals with a SADDLEBAG!")

this is live ("You cannot capture animals with a SADDLEBAG!")
flux closed this issue 2023-11-16 22:54:33 +00:00
Sign in to join this conversation.
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: your-land/bugtracker#4661
No description provided.