diff --git a/ChangeLog b/ChangeLog index 34fd60d..4811644 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,15 @@ Version 1.6.16 ******************************************************************************* +2012-03-06 Fabrice Fontaine + + SF Bug Tracker id 3497714 - Buffer overflows + + Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-06 07:36:08 PST + + Call to strcpy should be replaced by call to memset and strncpy to + avoid getting buffer overflows. + 2012-03-05 Marcelo Roberto Jimenez SF Bug Tracker id 2989399 - UpnpSetVirtualDirCallbacks API removal in 1.6.x diff --git a/upnp/src/api/upnpapi.c b/upnp/src/api/upnpapi.c index 22066a9..72e78d9 100644 --- a/upnp/src/api/upnpapi.c +++ b/upnp/src/api/upnpapi.c @@ -784,6 +784,7 @@ int UpnpRegisterRootDevice( retVal = UPNP_E_OUTOF_MEMORY; goto exit_function; } + memset(HInfo, 0, sizeof(struct Handle_Info)); HandleTable[*Hnd] = HInfo; UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, @@ -791,8 +792,8 @@ int UpnpRegisterRootDevice( HInfo->aliasInstalled = 0; HInfo->HType = HND_DEVICE; - strcpy(HInfo->DescURL, DescUrl); - strcpy(HInfo->LowerDescURL, DescUrl); + strncpy(HInfo->DescURL, DescUrl, sizeof(HInfo->DescURL) - 1); + strncpy(HInfo->LowerDescURL, DescUrl, sizeof(HInfo->LowerDescURL) - 1); UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Following Root Device URL will be used when answering to legacy CPs %s\n", HInfo->LowerDescURL); @@ -945,6 +946,7 @@ int UpnpRegisterRootDevice2( retVal = UPNP_E_OUTOF_MEMORY; goto exit_function; } + memset(HInfo, 0, sizeof(struct Handle_Info)); HandleTable[*Hnd] = HInfo; /* prevent accidental removal of a non-existent alias */ @@ -959,7 +961,8 @@ int UpnpRegisterRootDevice2( goto exit_function; } - strcpy(HInfo->LowerDescURL, HInfo->DescURL); + strncpy(HInfo->LowerDescURL, HInfo->DescURL, + sizeof(HInfo->LowerDescURL) - 1); UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Following Root Device URL will be used when answering to legacy CPs %s\n", HInfo->LowerDescURL); @@ -1110,16 +1113,19 @@ int UpnpRegisterRootDevice4( retVal = UPNP_E_OUTOF_MEMORY; goto exit_function; } + memset(HInfo, 0, sizeof(struct Handle_Info)); HandleTable[*Hnd] = HInfo; UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Root device URL is %s\n", DescUrl); HInfo->aliasInstalled = 0; HInfo->HType = HND_DEVICE; - strcpy(HInfo->DescURL, DescUrl); + strncpy(HInfo->DescURL, DescUrl, sizeof(HInfo->DescURL) - 1); if (LowerDescUrl == NULL) - strcpy(HInfo->LowerDescURL, DescUrl); + strncpy(HInfo->LowerDescURL, DescUrl, + sizeof(HInfo->LowerDescURL) - 1); else - strcpy(HInfo->LowerDescURL, LowerDescUrl); + strncpy(HInfo->LowerDescURL, LowerDescUrl, + sizeof(HInfo->LowerDescURL) - 1); UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Following Root Device URL will be used when answering to legacy CPs %s\n", HInfo->LowerDescURL); @@ -1871,10 +1877,11 @@ int UpnpSubscribeAsync( if( Param == NULL ) { return UPNP_E_OUTOF_MEMORY; } + memset( Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = SUBSCRIBE; Param->Handle = Hnd; - strcpy( Param->Url, EvtUrl ); + strncpy( Param->Url, EvtUrl, sizeof( Param->Url ) - 1 ); Param->TimeOut = TimeOut; Param->Fun = Fun; Param->Cookie = (void *)Cookie_const; @@ -2048,10 +2055,11 @@ int UpnpUnSubscribeAsync( retVal = UPNP_E_OUTOF_MEMORY; goto exit_function; } + memset( Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = UNSUBSCRIBE; Param->Handle = Hnd; - strcpy( Param->SubsId, SubsId ); + strncpy( Param->SubsId, SubsId, sizeof( Param->SubsId ) - 1 ); Param->Fun = Fun; Param->Cookie = (void *)Cookie_const; TPJobInit( &job, ( start_routine ) UpnpThreadDistribution, Param ); @@ -2164,10 +2172,11 @@ int UpnpRenewSubscriptionAsync( if( Param == NULL ) { return UPNP_E_OUTOF_MEMORY; } + memset(Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = RENEW; Param->Handle = Hnd; - strcpy( Param->SubsId, SubsId ); + strncpy( Param->SubsId, SubsId, sizeof( Param->SubsId ) - 1 ); Param->Fun = Fun; Param->Cookie = ( void * )Cookie_const; Param->TimeOut = TimeOut; @@ -2599,11 +2608,13 @@ int UpnpSendActionAsync( if( Param == NULL ) { return UPNP_E_OUTOF_MEMORY; } + memset( Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = ACTION; Param->Handle = Hnd; - strcpy( Param->Url, ActionURL ); - strcpy( Param->ServiceType, ServiceType ); + strncpy( Param->Url, ActionURL, sizeof ( Param->Url ) - 1 ); + strncpy( Param->ServiceType, ServiceType, + sizeof ( Param->ServiceType ) - 1 ); rc = ixmlParseBufferEx( tmpStr, &( Param->Act ) ); if( rc != IXML_SUCCESS ) { @@ -2694,11 +2705,13 @@ int UpnpSendActionExAsync( if( Param == NULL ) { return UPNP_E_OUTOF_MEMORY; } + memset( Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = ACTION; Param->Handle = Hnd; - strcpy( Param->Url, ActionURL ); - strcpy( Param->ServiceType, ServiceType ); + strncpy( Param->Url, ActionURL, sizeof( Param->Url ) - 1 ); + strncpy( Param->ServiceType, ServiceType, + sizeof ( Param->ServiceType ) - 1 ); retVal = ixmlParseBufferEx( headerStr, &( Param->Header ) ); if( retVal != IXML_SUCCESS ) { ixmlFreeDOMString( tmpStr ); @@ -2783,11 +2796,12 @@ int UpnpGetServiceVarStatusAsync( if( Param == NULL ) { return UPNP_E_OUTOF_MEMORY; } + memset( Param, 0, sizeof( struct UpnpNonblockParam ) ); Param->FunName = STATUS; Param->Handle = Hnd; - strcpy( Param->Url, ActionURL ); - strcpy( Param->VarName, VarName ); + strncpy( Param->Url, ActionURL, sizeof( Param->Url ) - 1); + strncpy( Param->VarName, VarName, sizeof( Param->VarName ) - 1 ); Param->Fun = Fun; Param->Cookie = ( void * )Cookie_const; @@ -3434,6 +3448,7 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) #if EXCLUDE_GENA == 0 case SUBSCRIBE: { struct Upnp_Event_Subscribe Evt; + memset(&Evt, 0, sizeof(Evt)); /* Cast away constness */ /*UpnpString *Sid = (UpnpString *)UpnpEventSubscribe_get_SID(evt);*/ UpnpString *Sid = UpnpString_new(); @@ -3444,9 +3459,11 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) Url, (int *)&Param->TimeOut, Sid); - strcpy(Evt.PublisherUrl, Param->Url); + strncpy(Evt.PublisherUrl, Param->Url, + sizeof(Evt.PublisherUrl) - 1); Evt.TimeOut = Param->TimeOut; - strcpy((char *)Evt.Sid, UpnpString_get_String(Sid)); + strncpy((char *)Evt.Sid, UpnpString_get_String(Sid), + sizeof((char *)Evt.Sid) - 1); Param->Fun(UPNP_EVENT_SUBSCRIBE_COMPLETE, &Evt, Param->Cookie); UpnpString_delete(Sid); UpnpString_delete(Url); @@ -3455,13 +3472,15 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) } case UNSUBSCRIBE: { struct Upnp_Event_Subscribe Evt; + memset(&Evt, 0, sizeof(Evt)); UpnpString *Sid = UpnpString_new(); UpnpString_set_String(Sid, Param->SubsId); Evt.ErrCode = genaUnSubscribe( Param->Handle, Sid); - strcpy((char *)Evt.Sid, UpnpString_get_String(Sid)); - strcpy(Evt.PublisherUrl, ""); + strncpy((char *)Evt.Sid, UpnpString_get_String(Sid), + sizeof((char *)Evt.Sid) - 1); + strncpy(Evt.PublisherUrl, "", sizeof(Evt.PublisherUrl) - 1); Evt.TimeOut = 0; Param->Fun(UPNP_EVENT_UNSUBSCRIBE_COMPLETE, &Evt, Param->Cookie); UpnpString_delete(Sid); @@ -3470,6 +3489,7 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) } case RENEW: { struct Upnp_Event_Subscribe Evt; + memset(&Evt, 0, sizeof(Evt)); UpnpString *Sid = UpnpString_new(); UpnpString_set_String(Sid, Param->SubsId); Evt.ErrCode = genaRenewSubscription( @@ -3477,7 +3497,8 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) Sid, &Param->TimeOut); Evt.TimeOut = Param->TimeOut; - strcpy((char *)Evt.Sid, UpnpString_get_String(Sid)); + strncpy((char *)Evt.Sid, UpnpString_get_String(Sid), + sizeof((char *)Evt.Sid) - 1); Param->Fun(UPNP_EVENT_RENEWAL_COMPLETE, &Evt, Param->Cookie); UpnpString_delete(Sid); free(Param); @@ -3487,13 +3508,14 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) #if EXCLUDE_SOAP == 0 case ACTION: { struct Upnp_Action_Complete Evt; + memset(&Evt, 0, sizeof(Evt)); Evt.ActionResult = NULL; Evt.ErrCode = SoapSendAction( Param->Url, Param->ServiceType, Param->Act, &Evt.ActionResult); Evt.ActionRequest = Param->Act; - strcpy(Evt.CtrlUrl, Param->Url); + strncpy(Evt.CtrlUrl, Param->Url, sizeof(Evt.CtrlUrl) - 1); Param->Fun(UPNP_CONTROL_ACTION_COMPLETE, &Evt, Param->Cookie); ixmlDocument_free(Evt.ActionRequest); ixmlDocument_free(Evt.ActionResult); @@ -3502,12 +3524,14 @@ void UpnpThreadDistribution(struct UpnpNonblockParam *Param) } case STATUS: { struct Upnp_State_Var_Complete Evt; + memset(&Evt, 0, sizeof(Evt)); Evt.ErrCode = SoapGetServiceVarStatus( Param->Url, Param->VarName, &Evt.CurrentVal); - strcpy(Evt.StateVarName, Param->VarName); - strcpy(Evt.CtrlUrl, Param->Url); + strncpy(Evt.StateVarName, Param->VarName, + sizeof(Evt.StateVarName) - 1); + strncpy(Evt.CtrlUrl, Param->Url, sizeof(Evt.CtrlUrl) - 1); Param->Fun(UPNP_CONTROL_GET_VAR_COMPLETE, &Evt, Param->Cookie); free(Evt.CurrentVal); free(Param); diff --git a/upnp/src/gena/gena_ctrlpt.c b/upnp/src/gena/gena_ctrlpt.c index 2d71095..768c6cb 100644 --- a/upnp/src/gena/gena_ctrlpt.c +++ b/upnp/src/gena/gena_ctrlpt.c @@ -155,6 +155,7 @@ static int ScheduleGenaAutoRenew( return_code = UPNP_E_OUTOF_MEMORY; goto end_function; } + memset(RenewEventStruct, 0, sizeof(struct Upnp_Event_Subscribe)); RenewEvent = (upnp_timeout *) malloc(sizeof(upnp_timeout)); if (RenewEvent == NULL) { @@ -162,11 +163,13 @@ static int ScheduleGenaAutoRenew( return_code = UPNP_E_OUTOF_MEMORY; goto end_function; } + memset(RenewEvent, 0, sizeof(upnp_timeout)); /* schedule expire event */ RenewEventStruct->ErrCode = UPNP_E_SUCCESS; RenewEventStruct->TimeOut = TimeOut; - strcpy(RenewEventStruct->Sid, UpnpString_get_String(tmpSID)); + strncpy(RenewEventStruct->Sid, UpnpString_get_String(tmpSID), + sizeof(RenewEventStruct->Sid) - 1); strncpy(RenewEventStruct->PublisherUrl, UpnpString_get_String(tmpEventURL), NAME_SIZE - 1); @@ -791,7 +794,9 @@ void gena_process_notification_event( /* fill event struct */ tmpSID = UpnpClientSubscription_get_SID(subscription); - strcpy(event_struct.Sid, UpnpString_get_String(tmpSID)); + memset(event_struct.Sid, 0, sizeof(event_struct.Sid)); + strncpy(event_struct.Sid, UpnpString_get_String(tmpSID), + sizeof(event_struct.Sid) - 1); event_struct.EventKey = eventKey; event_struct.ChangedVariables = ChangedVars; diff --git a/upnp/src/gena/gena_device.c b/upnp/src/gena/gena_device.c index 2c4d43c..4f3e355 100644 --- a/upnp/src/gena/gena_device.c +++ b/upnp/src/gena/gena_device.c @@ -558,7 +558,9 @@ int genaInitNotify( thread_struct->UDN = UDN_copy; thread_struct->headers = headers; thread_struct->propertySet = propertySet; - strcpy(thread_struct->sid, sid); + memset(thread_struct->sid, 0, sizeof(thread_struct->sid)); + strncpy(thread_struct->sid, sid, + sizeof(thread_struct->sid) - 1); thread_struct->eventKey = sub->eventKey++; thread_struct->reference_count = reference_count; thread_struct->device_handle = device_handle; @@ -714,7 +716,9 @@ int genaInitNotifyExt( thread_struct->UDN = UDN_copy; thread_struct->headers = headers; thread_struct->propertySet = propertySet; - strcpy(thread_struct->sid, sid); + memset(thread_struct->sid, 0, sizeof(thread_struct->sid)); + strncpy(thread_struct->sid, sid, + sizeof(thread_struct->sid) - 1); thread_struct->eventKey = sub->eventKey++; thread_struct->reference_count = reference_count; thread_struct->device_handle = device_handle; @@ -846,7 +850,10 @@ int genaNotifyAllExt( thread_struct->servId = servId_copy; thread_struct->headers = headers; thread_struct->propertySet = propertySet; - strcpy(thread_struct->sid, finger->sid); + memset(thread_struct->sid, 0, + sizeof(thread_struct->sid)); + strncpy(thread_struct->sid, finger->sid, + sizeof(thread_struct->sid) - 1); thread_struct->eventKey = finger->eventKey++; thread_struct->device_handle = device_handle; /* if overflow, wrap to 1 */ @@ -986,7 +993,10 @@ int genaNotifyAll( thread_struct->servId = servId_copy; thread_struct->headers = headers; thread_struct->propertySet = propertySet; - strcpy(thread_struct->sid, finger->sid); + memset(thread_struct->sid, 0, + sizeof(thread_struct->sid)); + strncpy(thread_struct->sid, finger->sid, + sizeof(thread_struct->sid) - 1); thread_struct->eventKey = finger->eventKey++; thread_struct->device_handle = device_handle; /* if overflow, wrap to 1 */ @@ -1196,6 +1206,8 @@ void gena_process_subscription_request( memptr callback_hdr; memptr timeout_hdr; + memset(&request_struct, 0, sizeof(request_struct)); + UpnpPrintf(UPNP_INFO, GENA, __FILE__, __LINE__, "Subscription Request Received:\n"); @@ -1341,7 +1353,8 @@ void gena_process_subscription_request( /* finally generate callback for init table dump */ request_struct.ServiceId = service->serviceId; request_struct.UDN = service->UDN; - strcpy((char *)request_struct.Sid, sub->sid); + strncpy((char *)request_struct.Sid, sub->sid, + sizeof(request_struct.Sid) - 1); /* copy callback */ callback_fun = handle_info->Callback; diff --git a/upnp/src/ssdp/ssdp_ctrlpt.c b/upnp/src/ssdp/ssdp_ctrlpt.c index 0e4a284..324149b 100644 --- a/upnp/src/ssdp/ssdp_ctrlpt.c +++ b/upnp/src/ssdp/ssdp_ctrlpt.c @@ -149,9 +149,9 @@ void ssdp_handle_ctrlpt_msg(http_message_t *hmsg, struct sockaddr_storage *dest_ linecopylen(param.Os, hdr_value.buf, hdr_value.length); } /* clear everything */ - param.DeviceId[0] = '\0'; - param.DeviceType[0] = '\0'; - param.ServiceType[0] = '\0'; + memset(param.DeviceId, 0, sizeof(param.DeviceId)); + memset(param.DeviceType, 0, sizeof(param.DeviceType)); + memset(param.ServiceType, 0, sizeof(param.ServiceType)); /* not used; version is in ServiceType */ param.ServiceVer[0] = '\0'; event.UDN[0] = '\0'; @@ -172,9 +172,11 @@ void ssdp_handle_ctrlpt_msg(http_message_t *hmsg, struct sockaddr_storage *dest_ hdr_value.buf[hdr_value.length] = save_char; } if (nt_found || usn_found) { - strcpy(param.DeviceId, event.UDN); - strcpy(param.DeviceType, event.DeviceType); - strcpy(param.ServiceType, event.ServiceType); + strncpy(param.DeviceId, event.UDN, sizeof(param.DeviceId) - 1); + strncpy(param.DeviceType, event.DeviceType, + sizeof(param.DeviceType) - 1); + strncpy(param.ServiceType, event.ServiceType, + sizeof(param.ServiceType) - 1); } /* ADVERT. OR BYEBYE */ if (hmsg->is_request) { diff --git a/upnp/src/ssdp/ssdp_server.c b/upnp/src/ssdp/ssdp_server.c index 47b78bd..7b494e3 100644 --- a/upnp/src/ssdp/ssdp_server.c +++ b/upnp/src/ssdp/ssdp_server.c @@ -110,6 +110,10 @@ int AdvertiseAndReply(int AdFlag, UpnpDevice_Handle Hnd, const DOMString dbgStr; int NumCopy = 0; + memset(UDNstr, 0, sizeof(UDNstr)); + memset(devType, 0, sizeof(devType)); + memset(servType, 0, sizeof(servType)); + UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Inside AdvertiseAndReply with AdFlag = %d\n", AdFlag); @@ -162,7 +166,7 @@ int AdvertiseAndReply(int AdFlag, UpnpDevice_Handle Hnd, tmpStr = ixmlNode_getNodeValue(textNode); if (!tmpStr) continue; - strcpy(devType, tmpStr); + strncpy(devType, tmpStr, sizeof(devType) - 1); UpnpPrintf(UPNP_ALL, API, __FILE__, __LINE__, "Extracting device type = %s\n", devType); if (!tmpNode) { @@ -197,7 +201,7 @@ int AdvertiseAndReply(int AdFlag, UpnpDevice_Handle Hnd, __LINE__, "UDN not found!\n"); continue; } - strcpy(UDNstr, tmpStr); + strncpy(UDNstr, tmpStr, sizeof(UDNstr) - 1); UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__, "Sending UDNStr = %s \n", UDNstr); if (AdFlag) { @@ -351,7 +355,7 @@ int AdvertiseAndReply(int AdFlag, UpnpDevice_Handle Hnd, tmpStr = ixmlNode_getNodeValue(textNode); if (!tmpStr) continue; - strcpy(servType, tmpStr); + strncpy(servType, tmpStr, sizeof(servType) - 1); UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__, "ServiceType = %s\n", servType); if (AdFlag) { @@ -483,19 +487,25 @@ int unique_service_name(char *cmd, SsdpEvent *Evt) n = (size_t) (Ptr - TempPtr); strncpy(Evt->UDN, TempPtr, n); Evt->UDN[n] = '\0'; - } else - strcpy(Evt->UDN, TempPtr); + } else { + memset(Evt->UDN, 0, sizeof(Evt->UDN)); + strncpy(Evt->UDN, TempPtr, sizeof(Evt->UDN) - 1); + } CommandFound = 1; } if (strstr(cmd, "urn:") != NULL && strstr(cmd, ":service:") != NULL) { if ((TempPtr = strstr(cmd, "urn")) != NULL) { - strcpy(Evt->ServiceType, TempPtr); + memset(Evt->ServiceType, 0, sizeof(Evt->ServiceType)); + strncpy(Evt->ServiceType, TempPtr, + sizeof(Evt->ServiceType) - 1); CommandFound = 1; } } if (strstr(cmd, "urn:") != NULL && strstr(cmd, ":device:") != NULL) { if ((TempPtr = strstr(cmd, "urn")) != NULL) { - strcpy(Evt->DeviceType, TempPtr); + memset(Evt->DeviceType, 0, sizeof(Evt->DeviceType)); + strncpy(Evt->DeviceType, TempPtr, + sizeof(Evt->DeviceType) - 1); CommandFound = 1; } }