Main: create Database on stack, move to Instance after Open() succeeded
This fixes use-after-free bug in SimpleDatabase::Close(), accessing the `root` object which was already freed by the `catch` block in Open(). By having the Database on the stack first, we can avoid calling Close() on the failed-to-open Database from Instance's destructor. Closes #482
This commit is contained in:
		
							
								
								
									
										20
									
								
								src/Main.cxx
									
									
									
									
									
								
							
							
						
						
									
										20
									
								
								src/Main.cxx
									
									
									
									
									
								
							@@ -185,18 +185,16 @@ InitStorage(const ConfigData &config, EventLoop &event_loop)
 | 
				
			|||||||
static bool
 | 
					static bool
 | 
				
			||||||
glue_db_init_and_load(const ConfigData &config)
 | 
					glue_db_init_and_load(const ConfigData &config)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	instance->database =
 | 
						auto db = CreateConfiguredDatabase(config, instance->event_loop,
 | 
				
			||||||
		CreateConfiguredDatabase(config, instance->event_loop,
 | 
					 | 
				
			||||||
					   instance->io_thread.GetEventLoop(),
 | 
										   instance->io_thread.GetEventLoop(),
 | 
				
			||||||
					   *instance);
 | 
										   *instance);
 | 
				
			||||||
	if (instance->database == nullptr)
 | 
						if (!db)
 | 
				
			||||||
		return true;
 | 
							return true;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (instance->database->GetPlugin().RequireStorage()) {
 | 
						if (db->GetPlugin().RequireStorage()) {
 | 
				
			||||||
		InitStorage(config, instance->io_thread.GetEventLoop());
 | 
							InitStorage(config, instance->io_thread.GetEventLoop());
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if (instance->storage == nullptr) {
 | 
							if (instance->storage == nullptr) {
 | 
				
			||||||
			instance->database.reset();
 | 
					 | 
				
			||||||
			LogDefault(config_domain,
 | 
								LogDefault(config_domain,
 | 
				
			||||||
				   "Found database setting without "
 | 
									   "Found database setting without "
 | 
				
			||||||
				   "music_directory - disabling database");
 | 
									   "music_directory - disabling database");
 | 
				
			||||||
@@ -210,22 +208,24 @@ glue_db_init_and_load(const ConfigData &config)
 | 
				
			|||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	try {
 | 
						try {
 | 
				
			||||||
		instance->database->Open();
 | 
							db->Open();
 | 
				
			||||||
	} catch (...) {
 | 
						} catch (...) {
 | 
				
			||||||
		std::throw_with_nested(std::runtime_error("Failed to open database plugin"));
 | 
							std::throw_with_nested(std::runtime_error("Failed to open database plugin"));
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	auto *db = dynamic_cast<SimpleDatabase *>(instance->database.get());
 | 
						instance->database = std::move(db);
 | 
				
			||||||
	if (db == nullptr)
 | 
					
 | 
				
			||||||
 | 
						auto *sdb = dynamic_cast<SimpleDatabase *>(instance->database.get());
 | 
				
			||||||
 | 
						if (sdb == nullptr)
 | 
				
			||||||
		return true;
 | 
							return true;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	instance->update = new UpdateService(config,
 | 
						instance->update = new UpdateService(config,
 | 
				
			||||||
					     instance->event_loop, *db,
 | 
										     instance->event_loop, *sdb,
 | 
				
			||||||
					     static_cast<CompositeStorage &>(*instance->storage),
 | 
										     static_cast<CompositeStorage &>(*instance->storage),
 | 
				
			||||||
					     *instance);
 | 
										     *instance);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* run database update after daemonization? */
 | 
						/* run database update after daemonization? */
 | 
				
			||||||
	return db->FileExists();
 | 
						return sdb->FileExists();
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static bool
 | 
					static bool
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user