Maniphest T96691

Heap corruption in file_browse_exec
Closed, ResolvedBUG
Actions

Assigned To
Campbell Barton (campbellbarton)
Authored By
Tom Edwards (artfunkel)
Mar 21 2022, 10:25 PM
Tags
  • User Interface
  • BF Blender (3.1)
Subscribers
Bastien Montagne (mont29)
Campbell Barton (campbellbarton)
Germano Cavalcante (mano-wii)
Tom Edwards (artfunkel)

Description

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.blend723 KBDownload

  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.

Revisions and Commits

rB Blender
rB9f15ee3c7ae0 Fix T96691: Heap corruption in file_browse_exec
rB87d9d33c0066 Fix T96691: Heap corruption in file_browse_exec

Related Objects

Mentioned In
T96241: 3.1: Potential candidates for corrective releases
P2839 3.1 R
Mentioned Here
rB0682af0d63a4: RNA: add length augmented to RNA_string_get_alloc

Event Timeline

Tom Edwards (artfunkel) created this task.Mar 21 2022, 10:25 PM
Germano Cavalcante (mano-wii) changed the task status from Needs Triage to Confirmed.Mar 22 2022, 4:52 PM
Germano Cavalcante (mano-wii) changed the subtype of this task from "Report" to "Bug".
Germano Cavalcante (mano-wii) added a project: Core.
Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).

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 (mont29) edited projects, added User Interface, BF Blender (3.1); removed Core, BF Blender.Mar 22 2022, 4:59 PM
Bastien Montagne (mont29) added subscribers: Campbell Barton (campbellbarton), Bastien Montagne (mont29).Mar 22 2022, 5:03 PM

@Campbell Barton (campbellbarton) rB0682af0d63a seems to be the direct cause of this issue, mind checking? thanks.

Bastien Montagne (mont29) triaged this task as High priority.Mar 22 2022, 5:03 PM
Bastien Montagne (mont29) moved this task from Backlog to bcon3: Bugs on the BF Blender (3.1) board.
Pratik Borhade (PratikPB2123) mentioned this in P2839 3.1 R.Mar 28 2022, 12:02 PM
Campbell Barton (campbellbarton) mentioned this in T96241: 3.1: Potential candidates for corrective releases.Mar 29 2022, 2:48 AM
Campbell Barton (campbellbarton) closed this task as Resolved by committing rB87d9d33c0066: Fix T96691: Heap corruption in file_browse_exec.Mar 29 2022, 2:49 AM
Campbell Barton (campbellbarton) claimed this task.
Campbell Barton (campbellbarton) added a commit: rB87d9d33c0066: Fix T96691: Heap corruption in file_browse_exec.
Philipp Oeser (lichtwerk) added a commit: rB9f15ee3c7ae0: Fix T96691: Heap corruption in file_browse_exec.Mar 29 2022, 4:35 PM