patches/24.aliasf_leak.patch

Change 541956399 by sungyc@sungyc:fig-export-icing-153-change-355:5516:citc on 2023/06/20 09:47:05

	[hunspell] Fix aliasf memory leak error
	
	## Test plan
	```
	sso_client -location 'https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=5175740794273792' > /tmp/testcase-5175740794273792 && \
	blaze --blazerc=/dev/null test --config=asan-fuzzer --test_strategy=local --test_sharding_strategy=disabled \
	  --test_env=ENABLE_BLAZE_TEST_FUZZING=1 --test_arg=-rss_limit_mb=2560 --test_arg=-timeout=90 --test_arg=-runs=100 --test_arg=/tmp/testcase-5175740794273792 \
	  //third_party/hunspell/fuzzers:dict_fuzzer
	```
	
	## Description
	The memory leak is caused by:
	- Some previous round allocated `aliasf[j]` successfully.
	- If later fails, then we free the entire `aliasf`/`aliasflen` and early return. But the original free memory code didn't correctly handle 2nd level of memory (`aliasf[j]`) allocated previously.
	
	This CL creates a new private method `free_aliasf` to handle all these cases.
	
	PRESUBMIT=passed
	BUG=280296690
	R=tjbarron
	CC=mghiware
	APPROVED=tjbarron
	REQUIRED_REVIEW=1
	DELTA=74 (32 added, 31 deleted, 11 changed)
	DELTA_BY_EXTENSION=cxx=42,hxx=1
	OCL=538018307
	FIG_CHANGESET=dcae75cc8f1f8f5fb7ea69ec7f25b3e731f7b6df
	FIG_WORKSPACE=sungyc/153:icing
	MARKDOWN=true

Affected files ...

... //depot//src/hunspell/hashmgr.cxx#9 edit
... //depot//src/hunspell/hashmgr.hxx#2 edit

==== //depot//src/hunspell/hashmgr.cxx#8 - /google/src/files/541956399/depot//src/hunspell/hashmgr.cxx ====
--- /google/src/files/537916729/depot//src/hunspell/hashmgr.cxx	2023-06-05 13:32:42.000000000 -0400
+++ /google/src/files/541956399/depot//src/hunspell/hashmgr.cxx	2023-06-20 12:47:05.000000000 -0400
@@ -30,6 +30,7 @@
   ignorechars_utf16_len = 0;
   numaliasf = 0;
   aliasf = NULL;
+  aliasflen = NULL;
   numaliasm = 0;
   aliasm = NULL;
   forbiddenword = FORBIDDENWORD; // forbidden word signing flag
@@ -66,19 +67,7 @@
   }
   tablesize = 0;
 
-  if (aliasf) {
-    for (int j = 0; j < (numaliasf); j++) {
-      if (aliasf[j]) {
-        free(aliasf[j]);
-      }
-    }
-    free(aliasf);
-    aliasf = NULL;
-    if (aliasflen) {
-      free(aliasflen);
-      aliasflen = NULL;
-    }
-  }
+  free_aliasf();
   if (aliasm) {
     for (int j = 0; j < (numaliasm); j++) {
       if (aliasm[j]) {
@@ -736,25 +725,27 @@
              case 1: { 
                        numaliasf = atoi(piece);
                        if (numaliasf < 1) {
-                          numaliasf = 0;
-                          aliasf = NULL;
-                          aliasflen = NULL;
+                          free_aliasf();
                           HUNSPELL_WARNING(stderr, "error: line %d: bad entry number\n", af->getlinenum());
                           return 1;
                        }
+
+                       // Allocate and initialize aliasf
                        aliasf = (unsigned short **) malloc(numaliasf * sizeof(unsigned short *));
-                       aliasflen = (unsigned short *) malloc(numaliasf * sizeof(short));
-                       if (!aliasf || !aliasflen) {
-                          numaliasf = 0;
-                          if (aliasf) free(aliasf);
-                          if (aliasflen) free(aliasflen);
-                          aliasf = NULL;
-                          aliasflen = NULL;
+                       if (!aliasf) {
+                          free_aliasf();
                           return 1;
                        }
-                       // Initialize all aliasf to NULL.
-                       memset(aliasf, 0, numaliasf * sizeof(unsigned short));
-                       memset(aliasflen, 0, numaliasf * sizeof(short));
+                       memset(aliasf, 0, numaliasf * sizeof(unsigned short *));
+
+                       // Allocate and initialize aliasflen
+                       aliasflen = (unsigned short *) malloc(numaliasf * sizeof(unsigned short));
+                       if (!aliasflen) {
+                          free_aliasf();
+                          return 1;
+                       }
+                       memset(aliasflen, 0, numaliasf * sizeof(unsigned short));
+
                        np++;
                        break;
                      }
@@ -765,11 +756,7 @@
        piece = mystrsep(&tp, 0);
    }
    if (np != 2) {
-      numaliasf = 0;
-      if (aliasf) free(aliasf);
-      if (aliasflen) free(aliasflen);
-      aliasf = NULL;
-      aliasflen = NULL;
+      free_aliasf();
       HUNSPELL_WARNING(stderr, "error: line %d: missing data\n", af->getlinenum());
       return 1;
    } 
@@ -777,7 +764,10 @@
    /* now parse the numaliasf lines to read in the remainder of the table */
    char * nl;
    for (int j=0; j < numaliasf; j++) {
-        if (!(nl = af->getline())) return 1;
+        if (!(nl = af->getline())) {
+          free_aliasf();
+          return 1;
+        }
         mychomp(nl);
         tp = nl;
         i = 0;
@@ -789,11 +779,7 @@
                switch(i) {
                   case 0: {
                              if (strncmp(piece,"AF",2) != 0) {
-                                 numaliasf = 0;
-                                 free(aliasf);
-                                 free(aliasflen);
-                                 aliasf = NULL;
-                                 aliasflen = NULL;
+                                 free_aliasf();
                                  HUNSPELL_WARNING(stderr, "error: line %d: table is corrupt\n", af->getlinenum());
                                  return 1;
                              }
@@ -803,6 +789,7 @@
                              int result_aflen =
                                  decode_flags(&(aliasf[j]), piece, af);
                              if (result_aflen == -1) {
+                                 free_aliasf();
                                  HUNSPELL_WARNING(
                                      stderr,
                                      "error: line %d: unable to decode flag or "
@@ -821,11 +808,7 @@
            piece = mystrsep(&tp, 0);
         }
         if (!aliasf[j]) {
-             free(aliasf);
-             free(aliasflen);
-             aliasf = NULL;
-             aliasflen = NULL;
-             numaliasf = 0;
+             free_aliasf();
              HUNSPELL_WARNING(stderr, "error: line %d: table is corrupt\n", af->getlinenum());
              return 1;
         }
@@ -959,3 +942,20 @@
     HUNSPELL_WARNING(stderr, "error: bad morph. alias index: %d\n", index);
     return NULL;
 }
+
+void HashMgr::free_aliasf() {
+    if (aliasf) {
+        for (int i = 0; i < numaliasf; i++) {
+             if (aliasf[i]) {
+               free(aliasf[i]);
+             }
+        }
+        free(aliasf);
+        aliasf = NULL;
+    }
+    if (aliasflen) {
+        free(aliasflen);
+        aliasflen = NULL;
+    }
+    numaliasf = 0;
+}
==== //depot//src/hunspell/hashmgr.hxx#1 - /google/src/files/541956399/depot//src/hunspell/hashmgr.hxx ====
--- /google/src/files/37375269/depot//src/hunspell/hashmgr.hxx	2012-10-31 14:23:51.000000000 -0400
+++ /google/src/files/541956399/depot//src/hunspell/hashmgr.hxx	2023-06-20 12:47:05.000000000 -0400
@@ -64,6 +64,7 @@
   int parse_aliasm(char * line, FileMgr * af);
   int remove_forbidden_flag(const char * word);
 
+  void free_aliasf();
 };
 
 #endif