Exploitable bug in tradeskill containers

[sfisque]

following code snippet (entire method) fixes an exploitable bug in tradeskill containers. replace the method in "tradeskills.cpp" with this one. (be careful, there are two methods with the same name, but different stack parameters)... enjoy

bool ZoneDatabase::GetTradeRecipe(const ItemInst* container, uint8 c_type, uint8 tradeskill, 
	DBTradeskillRecipe_Struct *spec)
{
	char errbuf[MYSQL_ERRMSG_SIZE];
    MYSQL_RES *result;
    MYSQL_ROW row;
    char *query = 0;
    char  where[ 10 ][ 128 ];
	char buf2[ 1024 ];
	
	int buf3[10];
	
	uint32 sum = 0;
	uint32 count = 0;
	uint32 qcount = 0;
	uint32 qlen = 0;
	
	//use the world item type as type if we have a world item
	//otherwise use the item's ID... this make the assumption that
	//no tradeskill containers will have an item ID which is
	//below the highest ID of objects, which is currently 0x30
	uint32 type = c_type;
	
	//dunno why I have to cast this up to call GetItem
	const Item_Struct *istruct = ((const ItemInst *) container)->GetItem();
	if(c_type == 0 && istruct) {
		type = istruct->ID;
	}
	
	uint8 i;

	for (i=0; i<10; i++) {
		const ItemInst* inst = container->GetItem(i);
		if (inst) {
			const Item_Struct* item = GetItem(inst->GetItem()->ID);
			if (item) {
					buf3[ count ++ ] = item->ID;
			}
		}
	}
	
	if(count < 1) {
		return(false);	//no items == no recipe
	}

	sprintf( buf2, "%s", "SELECT tre.recipe_id, count( tre.item_id ) "
	" FROM tradeskill_recipe_entries AS tre where tre.componentcount > 0" );
	
	for( i = 0; i < count; i ++ )
	{
		sprintf( where[ i ], " AND tre.recipe_id in ( select tre%d.recipe_id from "
			"tradeskill_recipe_entries tre%d where tre%d.item_id = %d )",
			i, i, i, buf3[ i ] );
		
		strcat( buf2, where[ i ] );
	}
	
	strcat( buf2, " group by tre.recipe_id" );
	
	qlen = MakeAnyLenString(&query, buf2 );	
	LogFile->write(EQEMuLog::Error, "Executing query: %s", query);

	if (!RunQuery(query, qlen, errbuf, &result)) {
		LogFile->write(EQEMuLog::Error, "Error in GetTradeRecept search, query: %s", query);
		safe_delete_array(query);
		LogFile->write(EQEMuLog::Error, "Error in GetTradeRecept search, error: %s", errbuf);
		return(false);
	}
	safe_delete_array(query);
	
	qcount = mysql_num_rows(result);
	if(qcount > 0) {
		//multiple recipes, partial match... do an extra query to get it exact.
		//this happens when combining components for a smaller recipe
		//which is completely contained within another recipe
		
		for (i = 0; i < qcount; i++) {
			row = mysql_fetch_row(result);
			int theCount = atoi( row[ 1 ] );
			if( theCount == count )
			{
				buf3[ sum ++ ] = atoi(row[0]);
			}
		}
		mysql_free_result(result);
	}

	if( sum != 1 ) {
		if( sum > 1 )
		{
			LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique!");
		}
		//else, just not found i guess..
		return(false);
	}
	
	return(GetTradeRecipe(buf3[ 0 ], c_type, tradeskill, spec));
}

== sfisque

[sfisque]

replace
sprintf( buf2, "%s", "SELECT tre.recipe_id, count( tre.item_id ) "

with
sprintf( buf2, "%s", "SELECT tre.recipe_id, sum( tre.componentcount ) "

the first line was too restrictive and didnt allow for "experimentation" with recipes that require multiples of the same item (studs for example, which require 3 metal bits).

the new query will allow for those, but there is a very small chance on false positives for recipes that have the exact same ingredients but only vary on the specific quantities of each ingredient but whose sum of ingredients is the same. if you find any such combines, post them to the forum so that we can work out a fix for it.

== sfisque

[sfisque]

i have a real fix for it. just have to do some more rigorous testing and then i'll post it up here.

== sfisque

[sfisque]

two phase fix. first update your database by running the sql here:

--
--
-- this adds the new column that allows us to optimize the "find the recipe" algorithm
--
--
alter table tradeskill_recipe add column component_sum int( 16 );

--
--
-- create temporary data for backfilling the new column
--
--
create temporary table recipe_result  ( select recipe_id, sum( item_id * componentcount ) as theSum from tradeskill_recipe_entries where componentcount > 0 group by recipe_id order by theSum );

--
--
-- this populates the new column with the derived data from the "entries" table
--
--
update tradeskill_recipe set component_sum = ( select theSum from recipe_result where recipe_result.recipe_id = tradeskill_recipe.id );

then patch the first GetTradeRecipe() method with this:

bool ZoneDatabase::GetTradeRecipe(const ItemInst* container, uint8 c_type, uint8 tradeskill, 
	DBTradeskillRecipe_Struct *spec)
{
	char errbuf[MYSQL_ERRMSG_SIZE];
    MYSQL_RES *result;
    MYSQL_ROW row;
    char *query = 0;
    char  where[ 10 ][ 16 ];
	char buf2[ 1024 ];
	char crcStr[ 32 ];
	
	int buf3[10];
	
	uint32 crc = 0;
	uint32 sum = 0;
	uint32 count = 0;
	uint32 qcount = 0;
	uint32 qlen = 0;
	uint32 theRecipe = 0;
	
	//use the world item type as type if we have a world item
	//otherwise use the item's ID... this make the assumption that
	//no tradeskill containers will have an item ID which is
	//below the highest ID of objects, which is currently 0x30
	uint32 type = c_type;
	
	//dunno why I have to cast this up to call GetItem
	const Item_Struct *istruct = ((const ItemInst *) container)->GetItem();
	if(c_type == 0 && istruct) {
		type = istruct->ID;
	}
	
	uint8 i;

	for (i=0; i<10; i++) {
		const ItemInst* inst = container->GetItem(i);
		if (inst) {
			const Item_Struct* item = GetItem(inst->GetItem()->ID);
			if (item) {
					buf3[ count ++ ] = item->ID;
					crc += item->ID;
			}
		}
	}
	
	if(count < 1) {
		return(false);	//no items == no recipe
	}

	sprintf( buf2, "%s", "select tre.recipe_id from"
		" tradeskill_recipe_entries tre, tradeskill_recipe where tre.item_id in ( " );
	for( i = 0; i < count; i ++ )
	{
		if( i != 0 )
		{
			sprintf( where[ i ], ", %d", buf3[ i ] );			
		}
		else
		{
			sprintf( where[ i ], "%d", buf3[ i ] );			
		}
		
		strcat( buf2, where[ i ] );
	}
	
	strcat( buf2, " ) and componentcount > 0 and tradeskill_recipe.id = tre.recipe_id"
		" and tradeskill_recipe.component_sum = " );
	sprintf( crcStr, "%d group by recipe_id", crc );
	strcat( buf2, crcStr );
	
	qlen = MakeAnyLenString(&query, buf2 );	
LogFile->write(EQEMuLog::Error, "Executing query: %s", query);

	if (!RunQuery(query, qlen, errbuf, &result)) {
		LogFile->write(EQEMuLog::Error, "Error in GetTradeRecept search, query: %s", query);
		safe_delete_array(query);
		LogFile->write(EQEMuLog::Error, "Error in GetTradeRecept search, error: %s", errbuf);
		return(false);
	}
	safe_delete_array(query);
	
	qcount = mysql_num_rows(result);

	if( qcount != 1 )
	{
		if( qcount > 1 )
		{
			LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique!");
		}
		//else, just not found i guess..
		return(false);
	}
	else
	{
		row = mysql_fetch_row(result);
		theRecipe = atoi( row[ 0 ] );
	}

	mysql_free_result(result);
	
	return(GetTradeRecipe(theRecipe, c_type, tradeskill, spec));
}