CVE-2023-42801: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: Moonlight-common-c contains the core GameStream client code shared between Moonlight clients. Moonlight-common-c is vulnerable to buffer overflow starting in commit f57bd745b4cbed577ea654fad4701bea4d38b44c. A malicious game streaming server could exploit a buffer overflow vulnerability to crash a moonlight client. Achieving RCE is possible but unlikely, due to stack canaries in use by modern compiler toolchains. The published binaries for official clients Qt, Android, iOS/tvOS, and Embedded are built with stack canaries, but some unofficial clients may not use stack canaries. This vulnerability takes place after the pairing process, so it requires the client to be tricked into pairing to a malicious host. It is not possible to perform using a man-in-the-middle due to public key pinning that takes place during the pairing process. The bug was addressed in commit b2497a3918a6d79808d9fd0c04734786e70d5954.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.6
Published: 2023-12-14T17:15Z
Repository: https://github.com/moonlight-stream/moonlight-common-c
Language: C
Files changed: src/Misc.c
Commit date: 2023-10-06 18:09:15 -0500
Commit message: Rewrite extractVersionQuadFromString() to avoid copying the input string

Diff: diff --git a/src/Misc.c b/src/Misc.c
index da56fa0..0df1ffc 100644
--- a/src/Misc.c
+++ b/src/Misc.c
@@ -78,36 +78,23 @@ int gracefullyDisconnectEnetPeer(ENetHost* host, ENetPeer* peer, enet_uint32 lin
         return 0;
     }
 }
 
 int extractVersionQuadFromString(const char* string, int* quad) {
-    char versionString[128];
-    char* nextDot;
-    char* nextNumber;
-    int i;
-    
-    strcpy(versionString, string);
-    nextNumber = versionString;
-    
-    for (i = 0; i < 4; i++) {
-        if (i == 3) {
-            nextDot = strchr(nextNumber, '\0');
-        }
-        else {
-            nextDot = strchr(nextNumber, '.');
-        }
-        if (nextDot == NULL) {
-            return -1;
+    const char* nextNumber = string;
+    for (int i = 0; i < 4; i++) {
+        // Parse the next component
+        quad[i] = (int)strtol(nextNumber, (char**)&nextNumber, 10);
+
+        // Skip the dot if we still have version components left.
+        //
+        // We continue looping even when we're at the end of the
+        // input string to ensure all subsequent version components
+        // are zeroed.
+        if (*nextNumber != 0) {
+            nextNumber++;
         }
-        
-        // Cut the string off at the next dot
-        *nextDot = '\0';
-        
-        quad[i] = atoi(nextNumber);
-        
-        // Move on to the next segment
-        nextNumber = nextDot + 1;
     }
     
     return 0;
 }
 

Vulnerable code:     char versionString[128];
    char* nextDot;
    char* nextNumber;
    int i;
    
    strcpy(versionString, string);
    nextNumber = versionString;
    
    for (i = 0; i < 4; i++) {
        if (i == 3) {
            nextDot = strchr(nextNumber, '\0');
        }
        else {
            nextDot = strchr(nextNumber, '.');
        }
        if (nextDot == NULL) {
            return -1;
        
        // Cut the string off at the next dot
        *nextDot = '\0';
        
        quad[i] = atoi(nextNumber);
        
        // Move on to the next segment
        nextNumber = nextDot + 1;
Fixed code:     const char* nextNumber = string;
    for (int i = 0; i < 4; i++) {
        // Parse the next component
        quad[i] = (int)strtol(nextNumber, (char**)&nextNumber, 10);

        // Skip the dot if we still have version components left.
        //
        // We continue looping even when we're at the end of the
        // input string to ensure all subsequent version components
        // are zeroed.
        if (*nextNumber != 0) {
            nextNumber++;