From 8b84d17c8d34e76d2531c6695d61349776474df2 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Fri, 6 Oct 2023 17:33:37 -0500 Subject: [PATCH] Replace additional unsafe string functions --- src/Platform.c | 34 ++++++++++++++++++++++++++++++++++ src/Platform.h | 1 + src/PlatformSockets.c | 6 +++--- src/PlatformSockets.h | 2 +- src/RtspConnection.c | 12 ++++++------ src/SdpGenerator.c | 9 ++++++--- src/SimpleStun.c | 2 +- 7 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/Platform.c b/src/Platform.c index 365ea77..7751a46 100644 --- a/src/Platform.c +++ b/src/Platform.c @@ -411,6 +411,40 @@ uint64_t PltGetMillis(void) { #endif } +bool PltSafeStrcpy(char* dest, size_t dest_size, const char* src) { + LC_ASSERT(dest_size > 0); + +#ifdef LC_DEBUG + // In debug builds, do the same little trick that MSVC + // does to ensure the entire buffer is writable. + memset(dest, 0xFE, dest_size); +#endif + +#ifdef _MSC_VER + // strncpy_s() with _TRUNCATE does what we need for MSVC. + // We use this rather than strcpy_s() because we don't want + // the invalid parameter handler invoked upon failure. + if (strncpy_s(dest, dest_size, src, _TRUNCATE) != 0) { + LC_ASSERT(false); + dest[0] = 0; + return false; + } +#else + // Check length of the source and destination strings before + // the strcpy() call. Set destination to an empty string if + // the source string doesn't fit in the destination. + if (strlen(src) >= dest_size) { + LC_ASSERT(false); + dest[0] = 0; + return false; + } + + strcpy(dest, src); +#endif + + return true; +} + int initializePlatform(void) { int err; diff --git a/src/Platform.h b/src/Platform.h index 0acd2e4..500081b 100644 --- a/src/Platform.h +++ b/src/Platform.h @@ -128,3 +128,4 @@ int initializePlatform(void); void cleanupPlatform(void); uint64_t PltGetMillis(void); +bool PltSafeStrcpy(char* dest, size_t dest_size, const char* src); diff --git a/src/PlatformSockets.c b/src/PlatformSockets.c index 25a6905..c26a969 100644 --- a/src/PlatformSockets.c +++ b/src/PlatformSockets.c @@ -33,7 +33,7 @@ DWORD (WINAPI *pfnWlanSetInterface)(HANDLE hClientHandle, CONST GUID *pInterface #endif -void addrToUrlSafeString(struct sockaddr_storage* addr, char* string) +void addrToUrlSafeString(struct sockaddr_storage* addr, char* string, size_t stringLength) { char addrstr[URLSAFESTRING_LEN]; @@ -43,7 +43,7 @@ void addrToUrlSafeString(struct sockaddr_storage* addr, char* string) inet_ntop(addr->ss_family, &sin6->sin6_addr, addrstr, sizeof(addrstr)); // IPv6 addresses need to be enclosed in brackets for URLs - sprintf(string, "[%s]", addrstr); + snprintf(string, stringLength, "[%s]", addrstr); } else #endif @@ -52,7 +52,7 @@ void addrToUrlSafeString(struct sockaddr_storage* addr, char* string) inet_ntop(addr->ss_family, &sin->sin_addr, addrstr, sizeof(addrstr)); // IPv4 addresses are returned without changes - sprintf(string, "%s", addrstr); + snprintf(string, stringLength, "%s", addrstr); } } diff --git a/src/PlatformSockets.h b/src/PlatformSockets.h index 5e72bbf..962cc6d 100644 --- a/src/PlatformSockets.h +++ b/src/PlatformSockets.h @@ -82,7 +82,7 @@ typedef struct sockaddr_in LC_SOCKADDR; #else #define URLSAFESTRING_LEN INET_ADDRSTRLEN #endif -void addrToUrlSafeString(struct sockaddr_storage* addr, char* string); +void addrToUrlSafeString(struct sockaddr_storage* addr, char* string, size_t stringLength); SOCKET createSocket(int addressFamily, int socketType, int protocol, bool nonBlocking); SOCKET connectTcpSocket(struct sockaddr_storage* dstaddr, SOCKADDR_LEN addrlen, unsigned short port, int timeoutSec); diff --git a/src/RtspConnection.c b/src/RtspConnection.c index eca0278..83a325e 100644 --- a/src/RtspConnection.c +++ b/src/RtspConnection.c @@ -72,8 +72,8 @@ static bool initializeRtspRequest(PRTSP_MESSAGE msg, char* command, char* target createRtspRequest(msg, NULL, 0, command, target, "RTSP/1.0", 0, NULL, NULL, 0); - sprintf(sequenceNumberStr, "%d", currentSeqNumber++); - sprintf(clientVersionStr, "%d", rtspClientVersion); + snprintf(sequenceNumberStr, sizeof(sequenceNumberStr), "%d", currentSeqNumber++); + snprintf(clientVersionStr, sizeof(clientVersionStr), "%d", rtspClientVersion); if (!addOption(msg, "CSeq", sequenceNumberStr) || !addOption(msg, "X-GS-ClientVersion", clientVersionStr) || (!useEnet && !addOption(msg, "Host", urlAddr))) { @@ -478,7 +478,7 @@ static bool sendVideoAnnounce(PRTSP_MESSAGE response, int* error) { request.flags |= FLAG_ALLOCATED_PAYLOAD; request.payloadLength = payloadLength; - sprintf(payloadLengthStr, "%d", payloadLength); + snprintf(payloadLengthStr, sizeof(payloadLengthStr), "%d", payloadLength); if (!addOption(&request, "Content-length", payloadLengthStr)) { goto FreeMessage; } @@ -582,7 +582,7 @@ static int parseOpusConfigurations(PRTSP_MESSAGE response) { channelCount = CHANNEL_COUNT_FROM_AUDIO_CONFIGURATION(StreamConfig.audioConfiguration); // Find the correct audio parameter value - sprintf(paramsPrefix, "a=fmtp:97 surround-params=%d", channelCount); + snprintf(paramsPrefix, sizeof(paramsPrefix), "a=fmtp:97 surround-params=%d", channelCount); paramStart = strstr(response->payload, paramsPrefix); if (paramStart) { // Skip the prefix @@ -801,12 +801,12 @@ int performRtspHandshake(PSERVER_INFORMATION serverInfo) { // NB: If the remote address is not a LAN address, the host will likely not enable high quality // audio since it only does that for local streaming normally. We can avoid this limitation, // but only if the caller gave us the RTSP session URL that it received from the host during launch. - addrToUrlSafeString(&RemoteAddr, urlAddr); + addrToUrlSafeString(&RemoteAddr, urlAddr, sizeof(urlAddr)); sprintf(rtspTargetUrl, "rtsp%s://%s:%u", useEnet ? "ru" : "", urlAddr, RtspPortNumber); } } else { - strcpy(urlAddr, "0.0.0.0"); + PltSafeStrcpy(urlAddr, sizeof(urlAddr), "0.0.0.0"); sprintf(rtspTargetUrl, "rtsp%s://%s:%u", useEnet ? "ru" : "", urlAddr, RtspPortNumber); } diff --git a/src/SdpGenerator.c b/src/SdpGenerator.c index 64466fb..c92f320 100644 --- a/src/SdpGenerator.c +++ b/src/SdpGenerator.c @@ -91,10 +91,13 @@ static int addAttributeBinary(PSDP_OPTION* head, char* name, const void* payload return -1; } + if (!PltSafeStrcpy(option->name, sizeof(option->name), name)) { + free(option); + return -1; + } + option->next = NULL; option->payloadLen = payloadLen; - strncpy(option->name, name, sizeof(option->name)); - option->name[sizeof(option->name) - 1] = '\0'; option->payload = (void*)(option + 1); memcpy(option->payload, payload, payloadLen); @@ -525,7 +528,7 @@ char* getSdpPayloadForStreamConfig(int rtspClientVersion, int* length) { char* payload; char urlSafeAddr[URLSAFESTRING_LEN]; - addrToUrlSafeString(&RemoteAddr, urlSafeAddr); + addrToUrlSafeString(&RemoteAddr, urlSafeAddr, sizeof(urlSafeAddr)); attributeList = getAttributesList(urlSafeAddr); if (attributeList == NULL) { diff --git a/src/SimpleStun.c b/src/SimpleStun.c index 56b5637..aab3a3e 100644 --- a/src/SimpleStun.c +++ b/src/SimpleStun.c @@ -65,7 +65,7 @@ int LiFindExternalAddressIP4(const char* stunServer, unsigned short stunPort, un hints.ai_protocol = IPPROTO_UDP; hints.ai_flags = AI_ADDRCONFIG; - sprintf(stunPortStr, "%u", stunPort); + snprintf(stunPortStr, sizeof(stunPortStr), "%u", stunPort); err = getaddrinfo(stunServer, stunPortStr, &hints, &stunAddrs); if (err != 0 || stunAddrs == NULL) { Limelog("Failed to resolve STUN server: %d\n", err);