AirConnect icon indicating copy to clipboard operation
AirConnect copied to clipboard

Heap overflow occur when acquiring "friendlyName" through XMLGetFirstDocumentItem()

Open firmianay opened this issue 2 years ago • 0 comments

hi ,great project!

I saw such a piece of code in airupnp/src/airupnp.c. It seems to read the field content from the configuration file. It is copied to Device->friendlyName (256 bytes) without checking its length, which may cause overflow. The scenario is that there is a malicious device with an overly long name. Or manually entered this super long string in the configuration file, the function is called in two places:

static bool AddMRDevice(struct sMR *Device, char *UDN, IXML_Document *DescDoc, const char *location)
{
...
	// Read key elements from description document
	friendlyName = XMLGetFirstDocumentItem(DescDoc, "friendlyName", true);
	if (friendlyName) strcpy(Device->friendlyName, friendlyName);
	if (!friendlyName || !*friendlyName) friendlyName = strdup(UDN);

	LOG_SDEBUG("UDN:\t%s\nFriendlyName:\t%s", UDN,  friendlyName);
static void *UpdateThread(void *args)
{
...
        // check for name change
        UpnpDownloadXmlDoc(Update->Data, &DescDoc);
        if (!friendlyName) friendlyName = XMLGetFirstDocumentItem(DescDoc, "friendlyName", true);
        
        if (friendlyName && strcmp(friendlyName, Device->friendlyName) &&
	        !strncmp(Device->friendlyName, Device->Config.Name, strlen(Device->friendlyName)) &&
	        Device->Config.Name[strlen(Device->Config.Name) - 1] == '+') {
	        LOG_INFO("[%p]: Device name change %s %s", Device, friendlyName, Device->friendlyName);
	        strcpy(Device->friendlyName, friendlyName);
	        sprintf(Device->Config.Name, "%s+", friendlyName);
	        raop_update(Device->Raop, Device->Config.Name, "airupnp");
	        Updated = true;
        }

firmianay avatar Jul 07 '22 12:07 firmianay