patches/19.alloc_fix.patch

Change 524965870 by sungyc@sungyc:fig-export-icing-153-change-311:4880:citc on 2023/04/17 15:28:39

	Fixes memory allocate when unable to parse test case.
	
	## Test plan
	```
	sso_client -location 'https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=5559624331558912' > /tmp/testcase-5559624331558912 && blaze --blazerc=/dev/null test -c opt --config=asan-fuzzer --test_strategy=local --test_sharding_strategy=disabled --test_env=ENABLE_BLAZE_TEST_FUZZING=1 --test_env=ASAN_OPTIONS="fast_unwind_on_fatal=0" --test_arg=-runs=100 --test_arg=/tmp/testcase-5559624331558912 //third_party/hunspell/fuzzers:dict_fuzzer
	```
	
	## Description
	This bug is caused by bad input.
	- In `HashMgr::decode_flags`:
	    - We call `u8_u16` to decode utf-8/16 characters.
	    - `u8_u16` returns the length of decoded string, but -1 if unable to decode.
	    - **Therefore, we have to check the returned length before calling malloc.**
	- Be aware of casting int to unsigned short.
	    - -1 will be casted to 65535.
	    - sizeof() returns an unsigned integer.
	    - So even if `len` is -1, calling `malloc(len * sizeof(...))` still attempts to allocate positive amount of memory (and may fail due to the # is too large).
	    - **Let's check `len` is NOT -1 before calling `malloc` to avoid this.**
	- Also we may fail to parse the input, or the input contains < `numaliasf`/`numaliasm` lines.
	    - For example, [this test case](https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=5559624331558912):
	        - Specifies 32764 af lines, but there is only one more line after this config.
	        - And the only line cannot be parsed by `u8_u16`.
	    - Which means, not all `aliasf[j]`s have been assigned a valid allocated memory address.
	    - **Thus, when freeing the memory, we should not assume all `aliasf[j]` is valid. Instead, we should:**
	        - **`memset()` all of them to 0 (NULL) when allocating.**
	        - **In destructor, only `free(aliasf[j])` if not NULL.**
	        - Same for `aliasm[j]`
	
	PRESUBMIT=passed
	BUG=273278397
	R=mghiware
	APPROVED=mghiware
	REQUIRED_REVIEW=1
	DELTA=33 (25 added, 0 deleted, 8 changed)
	DELTA_BY_EXTENSION=cxx=33
	OCL=524956186
	FIG_CHANGESET=3fff249fb23560092527bbc6ed5eb02b6c7dac2b
	FIG_WORKSPACE=sungyc/153:icing
	MARKDOWN=true

Affected files ...

... //depot//src/hunspell/hashmgr.cxx#6 edit

==== //depot//src/hunspell/hashmgr.cxx#5 - /google/src/files/524965870/depot//src/hunspell/hashmgr.cxx ====
--- /google/src/files/140493454/depot//src/hunspell/hashmgr.cxx	2016-11-29 13:12:47.000000000 -0500
+++ /google/src/files/524965870/depot//src/hunspell/hashmgr.cxx	2023-04-17 18:28:39.000000000 -0400
@@ -65,7 +65,11 @@
   tablesize = 0;
 
   if (aliasf) {
-    for (int j = 0; j < (numaliasf); j++) free(aliasf[j]);
+    for (int j = 0; j < (numaliasf); j++) {
+      if (aliasf[j]) {
+        free(aliasf[j]);
+      }
+    }
     free(aliasf);
     aliasf = NULL;
     if (aliasflen) {
@@ -74,7 +78,11 @@
     }
   }
   if (aliasm) {
-    for (int j = 0; j < (numaliasm); j++) free(aliasm[j]);
+    for (int j = 0; j < (numaliasm); j++) {
+      if (aliasm[j]) {
+        free(aliasm[j]);
+      }
+    }
     free(aliasm);
     aliasm = NULL;
   }  
@@ -540,6 +548,8 @@
       case FLAG_UNI: { // UTF-8 characters
         w_char w[MAXWORDLEN];
         len = u8_u16(w, MAXWORDLEN, flags);
+        // u8_u16 returns -1 when unable to decode.
+        if (len == -1) return -1;
         *result = (unsigned short *) malloc(len * sizeof(short));
         if (!*result) return -1;
         memcpy(*result, w, len * sizeof(short));
@@ -734,6 +744,9 @@
                           aliasflen = NULL;
                           return 1;
                        }
+                       // Initialize all aliasf to NULL.
+                       memset(aliasf, 0, numaliasf * sizeof(unsigned short));
+                       memset(aliasflen, 0, numaliasf * sizeof(short));
                        np++;
                        break;
                      }
@@ -745,8 +758,8 @@
    }
    if (np != 2) {
       numaliasf = 0;
-      free(aliasf);
-      free(aliasflen);
+      if (aliasf) free(aliasf);
+      if (aliasflen) free(aliasflen);
       aliasf = NULL;
       aliasflen = NULL;
       HUNSPELL_WARNING(stderr, "error: line %d: missing data\n", af->getlinenum());
@@ -779,9 +792,19 @@
                              break;
                           }
                   case 1: {
-                            aliasflen[j] = (unsigned short) decode_flags(&(aliasf[j]), piece, af);
-                            flag_qsort(aliasf[j], 0, aliasflen[j]);
-                            break; 
+                             int result_aflen =
+                                 decode_flags(&(aliasf[j]), piece, af);
+                             if (result_aflen == -1) {
+                                 HUNSPELL_WARNING(
+                                     stderr,
+                                     "error: line %d: unable to decode flag or "
+                                     "allocate memory\n",
+                                     af->getlinenum());
+                                 return 1;
+                             }
+                             aliasflen[j] = (unsigned short)result_aflen;
+                             flag_qsort(aliasf[j], 0, aliasflen[j]);
+                             break;
                           }
                   default: break;
                }
@@ -843,6 +866,8 @@
                           numaliasm = 0;
                           return 1;
                        }
+                       // Initialize all aliasm to NULL.
+                       memset(aliasm, 0, numaliasm * sizeof(char *));
                        np++;
                        break;
                      }
@@ -854,7 +879,7 @@
    }
    if (np != 2) {
       numaliasm = 0;
-      free(aliasm);
+      if (aliasm) free(aliasm);
       aliasm = NULL;
       HUNSPELL_WARNING(stderr, "error: line %d: missing data\n", af->getlinenum());
       return 1;