Heap corruption in file_browse_exec #96691

New Issue

Tom Edwards · 2022-03-21T22:25:07+01:00

Tom Edwards commented

2022-03-21 22:25:07 +01:00

System Information
Operating system: Windows 10
Graphics card: N/A

Blender Version
Broken: 3.1
Worked: 3.0.1

Short description of error
The method file_browse_exec in source\blender\editors\space_buttons\buttons_ops.c corrupts the heap when a relative directory path becomes longer than the absolute directory path.

The bug occurs between lines 210 and 213. path_len is the length of the absolute path (str), not the relative path (path) as seems to be intended.

The public release of Blender 3.1 suffers from these crashes, as do current builds of Git master.

The callstack:

ucrtbased.dll!free_dbg_nolock(void * const block, const int block_use) Line 952	C++
ucrtbased.dll!_free_dbg(void * block, int block_use) Line 1030	C++
ucrtbased.dll!free(void * block) Line 32	C++
blender.exe!MEM_lockfree_freeN(void * vmemh) Line 118	C
blender.exe!file_browse_exec(bContext * C, wmOperator * op) Line 231	C
blender.exe!wm_handler_fileselect_do(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, int val) Line 2571	C
blender.exe!wm_handler_fileselect_call(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, const wmEvent * event) Line 2670	C
blender.exe!wm_handlers_do_intern(bContext * C, wmWindow * win, wmEvent * event, ListBase * handlers) Line 3143	C
blender.exe!wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) Line 3199	C
blender.exe!wm_event_do_handlers(bContext * C) Line 3767	C
blender.exe!WM_main(bContext * C) Line 626	C
blender.exe!main(int argc, const unsigned char * * UNUSED_argv_c) Line 551	C

The debug assert message is:

HEAP CORRUPTION DETECTED: after Normal block (#2927924) at 0x0000014DE8153E80.
CRT detected that the application wrote to memory after end of heap buffer.

Exact steps for others to reproduce the error
file_select_heap_corruption.blend

Open the file above in Blender 3.1 and execute its startup script
Click on the file select dialog that appears in the bottom right corner of the screen (in "Custom Properties")
Click on the gear icon in the top right of the window that opens and check the "Relative Path" option
Select a path distant from the file, so that parent directory segments are required in the relative path (e.g. ../../../..)
Once you click Accept, heap corruption occurs.

**System Information** Operating system: Windows 10 Graphics card: N/A **Blender Version** Broken: 3.1 Worked: 3.0.1 **Short description of error** The method file_browse_exec in source\blender\editors\space_buttons\buttons_ops.c corrupts the heap when a relative directory path becomes longer than the absolute directory path. The bug occurs between lines 210 and 213. `path_len` is the length of the absolute path (`str`), not the relative path (`path`) as seems to be intended. The public release of Blender 3.1 suffers from these crashes, as do current builds of Git master. The callstack: ``` ucrtbased.dll!free_dbg_nolock(void * const block, const int block_use) Line 952 C++ ucrtbased.dll!_free_dbg(void * block, int block_use) Line 1030 C++ ucrtbased.dll!free(void * block) Line 32 C++ blender.exe!MEM_lockfree_freeN(void * vmemh) Line 118 C blender.exe!file_browse_exec(bContext * C, wmOperator * op) Line 231 C blender.exe!wm_handler_fileselect_do(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, int val) Line 2571 C blender.exe!wm_handler_fileselect_call(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, const wmEvent * event) Line 2670 C blender.exe!wm_handlers_do_intern(bContext * C, wmWindow * win, wmEvent * event, ListBase * handlers) Line 3143 C blender.exe!wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) Line 3199 C blender.exe!wm_event_do_handlers(bContext * C) Line 3767 C blender.exe!WM_main(bContext * C) Line 626 C blender.exe!main(int argc, const unsigned char * * UNUSED_argv_c) Line 551 C ``` The debug assert message is: > HEAP CORRUPTION DETECTED: after Normal block (#2927924) at 0x0000014DE8153E80. > CRT detected that the application wrote to memory after end of heap buffer. **Exact steps for others to reproduce the error** [file_select_heap_corruption.blend](https://archive.blender.org/developer/F12938904/file_select_heap_corruption.blend) 1. Open the file above in Blender 3.1 and execute its startup script 2. Click on the file select dialog that appears in the bottom right corner of the screen (in "Custom Properties") 3. Click on the gear icon in the top right of the window that opens and check the "Relative Path" option 4. Select a path distant from the file, so that parent directory segments are required in the relative path (e.g. ../../../..) 5. Once you click Accept, heap corruption occurs.

Tom Edwards commented

2022-03-21 22:25:07 +01:00

Added subscriber: @artfunkel

Germano Cavalcante commented

2022-03-22 16:52:53 +01:00

Added subscriber: @mano-wii

Germano Cavalcante commented

2022-03-22 16:52:53 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Germano Cavalcante commented

2022-03-22 16:52:54 +01:00

I can confirm the problem by generating this relative path: "//..\..\..\..\..\..\..\..\Germano\Instaladores\"
Investigating the code, it looks like something is really wrong. The BLI_path_slash_ensure call seems redundant, the path_len is taken with the path before being converted to relative and MEM_reallocN is called where MEM_freeN + MEM_mallocN seems to be the intent.

Here a quick solution:

diff --git a/source/blender/editors/space_buttons/buttons_ops.c b/source/blender/editors/space_buttons/buttons_ops.c
index f91ed5eb4f3..840d34584e7 100644
--- a/source/blender/editors/space_buttons/buttons_ops.c
+++ b/source/blender/editors/space_buttons/buttons_ops.c
@@ -204,12 +204,11 @@ static int file_browse_exec(bContext *C, wmOperator *op)
     BLI_path_abs(path, id ? ID_BLEND_PATH(bmain, id) : BKE_main_blendfile_path(bmain));
 
     if (BLI_is_dir(path)) {
-      /* Do this first so '*' isn't converted to '*\' on windows. */
-      BLI_path_slash_ensure(path);
       if (is_relative) {
-        const int path_len = BLI_strncpy_rlen(path, str, FILE_MAX);
+        BLI_strncpy(path, str, FILE_MAX);
         BLI_path_rel(path, BKE_main_blendfile_path(bmain));
+        const int path_len = BLI_strnlen(path, FILE_MAX - 1);
         str = MEM_reallocN(str, path_len + 2);
         BLI_strncpy(str, path, FILE_MAX);
       }
       else {

I can confirm the problem by generating this relative path: `"//..\..\..\..\..\..\..\..\Germano\Instaladores\"` Investigating the code, it looks like something is really wrong. The `BLI_path_slash_ensure` call seems redundant, the `path_len` is taken with the path before being converted to relative and `MEM_reallocN` is called where `MEM_freeN` + `MEM_mallocN` seems to be the intent. Here a quick solution: ``` diff --git a/source/blender/editors/space_buttons/buttons_ops.c b/source/blender/editors/space_buttons/buttons_ops.c index f91ed5eb4f3..840d34584e7 100644 --- a/source/blender/editors/space_buttons/buttons_ops.c +++ b/source/blender/editors/space_buttons/buttons_ops.c @@ -204,12 +204,11 @@ static int file_browse_exec(bContext *C, wmOperator *op) BLI_path_abs(path, id ? ID_BLEND_PATH(bmain, id) : BKE_main_blendfile_path(bmain)); if (BLI_is_dir(path)) { - /* Do this first so '*' isn't converted to '*\' on windows. */ - BLI_path_slash_ensure(path); if (is_relative) { - const int path_len = BLI_strncpy_rlen(path, str, FILE_MAX); + BLI_strncpy(path, str, FILE_MAX); BLI_path_rel(path, BKE_main_blendfile_path(bmain)); + const int path_len = BLI_strnlen(path, FILE_MAX - 1); str = MEM_reallocN(str, path_len + 2); BLI_strncpy(str, path, FILE_MAX); } else { ```

Bastien Montagne commented

2022-03-22 17:03:00 +01:00

Added subscribers: @ideasman42, @mont29