On Monday, February 27, 2017 at 1:30:41 AM UTC+8, Bram Moolenaar wrote: > shqking wrote: > > > One null pointer dereference vulnerability is found in src/undo.c > > (https://github.com/vim/vim/blob/master/src/undo.c) > > > > The code snippet is as follows. > > > > 1383 static u_entry_T * > > 1384 unserialize_uep(bufinfo_T *bi, int *error, char_u *file_name) > > 1385 { > > ... > > 1402 uep->ue_size = undo_read_4c(bi); > > 1403 if (uep->ue_size > 0) > > 1404 { > > 1405 array = (char_u **)U_ALLOC_LINE(sizeof(char_u *) * > > uep->ue_size); > > 1406 if (array == NULL) > > 1407 { > > 1408 *error = TRUE; > > 1409 return uep; > > 1410 } > > 1411 vim_memset(array, 0, sizeof(char_u *) * uep->ue_size); > > 1412 } > > 1413 else > > 1414 array = NULL; > > 1415 uep->ue_array = array; > > 1416 > > 1417 for (i = 0; i < uep->ue_size; ++i) > > .... > > 1434 return uep; > > 1435 } > > > > Variable uep->ue_size is read from user file at line 1402，so its value can > > be controlled by users. Null pointer is assigned to array at line 1414 if > > uep->ue_size is not positive, then uep->ue_array is also null at line 1415. > > After that, uep is returned from this function. > > > > Since no error code is set along this ELSE branch, the calling function > > unserialize_uhp() will link uep into the list uhp through the while-loop at > > line 1332. After that, uhp will be store into uhp_table at line 1991 in > > function u_read_undo, and uhp_table is referred via global pointer > > curbuf->b_u_curhead (at line 2085). > > > > As a result, null pointer dereference vulnerability would occur if the > > program reads the uhp_table and uses uep->ue_array， which might be null > > pointer. For example, one usage place is at line 2680 in function > > u_undoredo. > > > > One possible workaround is to return an error code for the ELSE branch at > > line 1413, just as line 1406 does. Attached please find the patch. > > It is OK for ue_array to be NULL if the size is zero (or negative). > All places where ue_array is accessed should check the value of ue_size. > I don't see such a place. Thus I don't think there is a problem. > > -- > How To Keep A Healthy Level Of Insanity: > 7. Finish all your sentences with "in accordance with the prophecy". > > /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\ > /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ > \\\ an exciting new programming language -- http://www.Zimbu.org /// > \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
Yes. After a more careful review of the source code, I agree with you that it is NOT a problem. Sorry for reporting such a false positive. -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
