Browse code

bb12515: Fix for out-of-bounds read in ARJ parser

Fix for an out-of-bounds read in the ARJ parser accidentally introduced
when adding text normalization and bound checking when parsing filename
and comment fields from file headers.

Micah Snyder (micasnyd) authored on 2020/04/08 04:20:39
Showing 1 changed files
... ...
@@ -834,18 +834,16 @@ static int arj_read_main_header(arj_metadata_t *metadata)
834 834
     uint16_t header_size, count;
835 835
     arj_main_hdr_t main_hdr;
836 836
     const char *filename = NULL;
837
-    const char *comment = NULL;
838
-    off_t header_offset;
837
+    const char *comment  = NULL;
839 838
     struct text_norm_state fnstate, comstate;
840
-    unsigned char *fnnorm = NULL;
839
+    unsigned char *fnnorm  = NULL;
841 840
     unsigned char *comnorm = NULL;
842
-    uint32_t ret = TRUE;
841
+    uint32_t ret           = TRUE;
843 842
 
844 843
     if (fmap_readn(metadata->map, &header_size, metadata->offset, 2) != 2)
845 844
         return FALSE;
846 845
 
847 846
     metadata->offset += 2;
848
-    header_offset = metadata->offset;
849 847
     header_size   = le16_to_host(header_size);
850 848
     cli_dbgmsg("Header Size: %d\n", header_size);
851 849
     if (header_size == 0) {
... ...
@@ -882,8 +880,8 @@ static int arj_read_main_header(arj_metadata_t *metadata)
882 882
         metadata->offset += main_hdr.first_hdr_size - 30;
883 883
     }
884 884
 
885
-    fnnorm = cli_calloc(sizeof(unsigned char), header_size + 1);
886
-    filename = fmap_need_offstr(metadata->map, metadata->offset, header_size);
885
+    fnnorm   = cli_calloc(sizeof(unsigned char), header_size + 1);
886
+    filename = fmap_need_offstr(metadata->map, metadata->offset, header_size + 1);
887 887
     if (!filename) {
888 888
         cli_dbgmsg("UNARJ: Unable to allocate memory for filename\n");
889 889
         ret = FALSE;
... ...
@@ -892,7 +890,7 @@ static int arj_read_main_header(arj_metadata_t *metadata)
892 892
     metadata->offset += CLI_STRNLEN(filename, header_size) + 1;
893 893
 
894 894
     comnorm = cli_calloc(sizeof(unsigned char), header_size + 1);
895
-    comment = fmap_need_offstr(metadata->map, metadata->offset, header_size);
895
+    comment = fmap_need_offstr(metadata->map, metadata->offset, header_size + 1);
896 896
     if (!comment || !comnorm) {
897 897
         cli_dbgmsg("UNARJ: Unable to allocate memory for comment\n");
898 898
         ret = FALSE;
... ...
@@ -903,8 +901,8 @@ static int arj_read_main_header(arj_metadata_t *metadata)
903 903
     text_normalize_init(&fnstate, fnnorm, header_size);
904 904
     text_normalize_init(&comstate, comnorm, header_size);
905 905
 
906
-    text_normalize_buffer(&fnstate, filename, metadata->offset);
907
-    text_normalize_buffer(&comstate, comment, metadata->offset);
906
+    text_normalize_buffer(&fnstate, (const unsigned char *)filename, header_size);
907
+    text_normalize_buffer(&comstate, (const unsigned char *)comment, header_size);
908 908
 
909 909
     cli_dbgmsg("Filename: %s\n", fnnorm);
910 910
     cli_dbgmsg("Comment: %s\n", comnorm);
... ...
@@ -947,9 +945,9 @@ static int arj_read_file_header(arj_metadata_t *metadata)
947 947
     const char *filename, *comment;
948 948
     arj_file_hdr_t file_hdr;
949 949
     struct text_norm_state fnstate, comstate;
950
-    unsigned char *fnnorm = NULL;
950
+    unsigned char *fnnorm  = NULL;
951 951
     unsigned char *comnorm = NULL;
952
-    uint32_t ret = CL_SUCCESS;
952
+    uint32_t ret           = CL_SUCCESS;
953 953
 
954 954
     if (fmap_readn(metadata->map, &header_size, metadata->offset, 2) != 2)
955 955
         return CL_EFORMAT;
... ...
@@ -999,8 +997,8 @@ static int arj_read_file_header(arj_metadata_t *metadata)
999 999
         metadata->offset += file_hdr.first_hdr_size - 30;
1000 1000
     }
1001 1001
 
1002
-    fnnorm = cli_calloc(sizeof(unsigned char), header_size + 1);
1003
-    filename = fmap_need_offstr(metadata->map, metadata->offset, header_size);
1002
+    fnnorm   = cli_calloc(sizeof(unsigned char), header_size + 1);
1003
+    filename = fmap_need_offstr(metadata->map, metadata->offset, header_size + 1);
1004 1004
     if (!filename) {
1005 1005
         cli_dbgmsg("UNARJ: Unable to allocate memory for filename\n");
1006 1006
         ret = FALSE;
... ...
@@ -1009,7 +1007,7 @@ static int arj_read_file_header(arj_metadata_t *metadata)
1009 1009
     metadata->offset += CLI_STRNLEN(filename, header_size) + 1;
1010 1010
 
1011 1011
     comnorm = cli_calloc(sizeof(unsigned char), header_size + 1);
1012
-    comment = fmap_need_offstr(metadata->map, metadata->offset, header_size);
1012
+    comment = fmap_need_offstr(metadata->map, metadata->offset, header_size + 1);
1013 1013
     if (!comment) {
1014 1014
         cli_dbgmsg("UNARJ: Unable to allocate memory for comment\n");
1015 1015
         ret = FALSE;
... ...
@@ -1020,8 +1018,8 @@ static int arj_read_file_header(arj_metadata_t *metadata)
1020 1020
     text_normalize_init(&fnstate, fnnorm, header_size);
1021 1021
     text_normalize_init(&comstate, comnorm, header_size);
1022 1022
 
1023
-    text_normalize_buffer(&fnstate, filename, metadata->offset);
1024
-    text_normalize_buffer(&comstate, comment, metadata->offset);
1023
+    text_normalize_buffer(&fnstate, (const unsigned char *)filename, header_size);
1024
+    text_normalize_buffer(&comstate, (const unsigned char *)comment, header_size);
1025 1025
 
1026 1026
     cli_dbgmsg("Filename: %s\n", fnnorm);
1027 1027
     cli_dbgmsg("Comment: %s\n", comnorm);
... ...
@@ -1037,7 +1035,7 @@ static int arj_read_file_header(arj_metadata_t *metadata)
1037 1037
             if (metadata->filename)
1038 1038
                 free(metadata->filename);
1039 1039
             metadata->filename = NULL;
1040
-            ret = CL_EFORMAT;
1040
+            ret                = CL_EFORMAT;
1041 1041
             goto done;
1042 1042
         }
1043 1043
         count = cli_readint16(countp);
... ...
@@ -1055,11 +1053,11 @@ static int arj_read_file_header(arj_metadata_t *metadata)
1055 1055
     metadata->encrypted = ((file_hdr.flags & GARBLE_FLAG) != 0) ? TRUE : FALSE;
1056 1056
     metadata->ofd       = -1;
1057 1057
     if (!metadata->filename) {
1058
-        ret =  CL_EMEM;
1058
+        ret = CL_EMEM;
1059 1059
         goto done;
1060 1060
     }
1061 1061
 
1062
-    done:
1062
+done:
1063 1063
 
1064 1064
     if (fnnorm) {
1065 1065
         free(fnnorm);