Skip to content

MCB corruption from mismatch in maximum size allocated for and read from flat-format .COM executable #207

Open
Open
MCB corruption from mismatch in maximum size allocated for and read from flat-format .COM executable#207
Labels
bugSomething isn't working

Description

@ecm-pushbx
ecm-pushbx
opened on Jun 3, 2025

Here's the Script for lDebug file that reliably causes the kernel to corrupt one of its MCBs (if an UMCB chain is present and UMB link state is disabled): https://pushbx.org/ecm/test/20250603/testbigc.sld (I will copy the contents into a subsequent comment in this thread.)

You run it like so:

ldebug
y testbigc.sld
g
dm
l
dm
q

What it does is it exposes a difference in the flat-fomat .COM style executable maximum size to allocate and to load, in task.c. The maximum required memory block is smaller than the maximum file data read, so with a certain range of free MCB size and executable size the allocation will be smaller than the amount of file data read, leading to corrupting memory beyond the end of the allocated memory block (most obviously corrupting the next MCB).

Flat-format executables sized >= FF00h bytes are rejected by the MS-DOS v4.01 and lDOS executable loaders. So the problem might be restricted to pathological cases of executable sizes. However, the other condition of this (precise free block size) is not purely pathological, it can occur when loading to UMBs using LOADHIGH.

Here's where the file size is determined to get the minimum size of memory allocation needed to run the program, it requires at most FE0h + 10h = FF0h paragraphs:

kernel/kernel/task.c

Lines 470 to 473 in b8ef68f

ULONG com_size_long = SftGetFsize(fd);
/* maximally 64k - 256 bytes stack -
256 bytes psp */
com_size = ((UWORD)min(com_size_long, 0xfe00u) >> 4) + 0x10;

Here's where the file is read, it reads up to FF00h bytes during exec (not overlay/device load) to offset 0100h hence writing to offsets 0100h .. FFFFh:

kernel/kernel/task.c

Lines 532 to 540 in b8ef68f

/* MS DOS always only loads the very first 64KB - sizeof(psp) bytes.
-- 1999/04/21 ska */
/* rewind to start */
SftSeek(fd, 0, 0);
/* read everything, but at most 64K - sizeof(PSP) */
/* lpproj: some device drivers (not exe) are larger than 0xff00bytes... */
DosRWSft(fd, (mode == OVERLAY) ? 0xfffeU : 0xff00U, sp, XFR_READ);
DosCloseSft(fd, FALSE);

Activity

  1. ecm-pushbx
    added
    bugSomething isn't working
    on Jun 3, 2025

  2. ecm-pushbx commented on Jun 3, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 3, 2025
    ContributorAuthor

    testbigc.sld file contents:

    if (v3 != 0) then y :freev3
if (v4 == 0) then r v4 := 0FF1
if (v5 == 0) then r v5 := FF00
k testbigc.com
f 100 FFFF CC
a 100
 retn
 .
r bxcx := v5
w 100
r byte [cs:ip + 1] := -1			; workaround
l
if (rc) then goto :eof				; this doesn't work really
if (byte [cs:ip + 1] != CC) then goto :eof	; workaround
f 100 l 400 CC
a 100
 mov ah, 4A
 mov bx, 1000
 int 21
 xor ax, ax
 xchg ax, [2C]
 dec ax
 mov es, ax
 mov bx, word [es:3]
 inc ax
 mov es, ax
 mov ah, 49
 int 21
 add bx, (v4)
 mov ah, 48
 int 21
 jc 180
 push ax
 .
r v0 := aao
a
 mov ah, 48
 mov bx, -1
 int 21
 mov ah, 48
 int 21
 jnc (v0)
 pop es
 mov ah, 49
 int 21
 xor bx, bx
 .
r v1 := aao
a
 mov ah, 3E
 int 21
 inc bx
 cmp bx, 20
 jb (v1)
 mov ax, 3100
 mov dx, 1000
 int 21
 .
r v3 := psp
goto :eof

:freev3
. push es
. push bx
. push ax
. mov ah, 52
. int 21
r v10 := word [es:bx - 2]
. pop ax
. pop bx
. pop es
r v11 := 0
:freeloop
if (! value byte [$v10:0] in #'M', #'Z') then goto :freeerror
if (word [$v10:1] == v3) then r word [$v10:1] := 0
if (byte [$v10:0] == #'Z') then goto :freeend
r v10 += word [$v10:3] + 1
r v11 += 1
if (v11 > FFFF) then goto :freeerror
goto :freeloop

:freeerror
; Error during MCB freeing!
goto :eof

:freeend
; Freed
r v3 := 0

  3. ecm-pushbx commented on Jun 3, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 3, 2025
    ContributorAuthor

    Test log:

    
&; Welcome to lDebug!
Press any key to enter debugger terminal!
Timer stopped
V0=000000A8 V1=00000000 V2=00000000 V3=00000000  DCO=00000000 DCS=00000000
V4=00000000 V5=00000000 V6=00000000 V7=00000000  DAO=0000F007 DAS=0000F007
V8=00000000 V9=00000000 VA=00000000 VB=00000000  DIF=0180F008 DPI=0000:0000
VC=00000000 VD=00000000 VE=00000000 VF=00000000  DPR=7681     DPP=0000
Current mode: Virtual 86 Mode
-fred
-q
123
FreeDOS kernel - GIT (build 2043 OEM:0xfd) [compiled Mar 17 2025]
Kernel compatibility 7.10 - GNUC - 808x compatible - FAT32 support

(C) Copyright 1995-2023 Pasquale J. Villani and The FreeDOS Project.
All Rights Reserved. This is free software and comes with ABSOLUTELY NO
WARRANTY; you can redistribute it and/or modify it under the terms of the
GNU General Public License as published by the Free Software Foundation;
either version 2, or (at your option) any later version.
C: HD1, Pri[ 1], CHS=    0-1-1, start=     0 MB, size=  2000 MB
D: HD2, Pri[ 1], CHS=    0-1-1, start=     0 MB, size=  2000 MB
dosemu XMS 3.0 & UMB support enabled
EMUFS host file and print access available
dosemu EMS driver rev 0.9 installed.
dosemu CDROM driver installed (V0.2)
Kernel: allocated 32 Diskbuffers = 17024 Bytes in HMA
emufs: redirector enabled

FreeCom version 0.87 - GNUC - XMS_Swap [Feb 21 2025 23:30:19]
SEEKEXT 1.21: high loaded using 416 byte, on multiplex 00h.
emufs: redirector enabled
Welcome to dosemu2!
    Build 2.0pre9-dev-20240420-1892-g6a2f4f527
C:\>ldebug
-y testbigc.sld
-if (v3 != 0) then y :freev3
-if (v4 == 0) then r v4 := 0FF1
-if (v5 == 0) then r v5 := FF00
-k testbigc.com
-f 100 FFFF CC
-a 100
2975:0100  retn
2975:0101  .
-r bxcx := v5
-w 100
Writing FF00 bytes
-r byte [cs:ip + 1] := -1                       ; workaround
-l
-if (rc) then goto :eof                         ; this doesn't work really
-if (byte [cs:ip + 1] != CC) then goto :eof     ; workaround
-f 100 l 400 CC
-a 100
2975:0100  mov ah, 4A
2975:0102  mov bx, 1000
2975:0105  int 21
2975:0107  xor ax, ax
2975:0109  xchg ax, [2C]
2975:010D  dec ax
2975:010E  mov es, ax
2975:0110  mov bx, word [es:3]
2975:0115  inc ax
2975:0116  mov es, ax
2975:0118  mov ah, 49
2975:011A  int 21
2975:011C  add bx, (v4)
2975:0120  mov ah, 48
2975:0122  int 21
2975:0124  jc 180
2975:0126  push ax
2975:0127  .
-r v0 := aao
-a
2975:0127  mov ah, 48
2975:0129  mov bx, -1
2975:012C  int 21
2975:012E  mov ah, 48
2975:0130  int 21
2975:0132  jnc (v0)
2975:0134  pop es
2975:0135  mov ah, 49
2975:0137  int 21
2975:0139  xor bx, bx
2975:013B  .
-r v1 := aao
-a
2975:013B  mov ah, 3E
2975:013D  int 21
2975:013F  inc bx
2975:0140  cmp bx, 20
2975:0143  jb (v1)
2975:0145  mov ax, 3100
2975:0148  mov dx, 1000
2975:014B  int 21
2975:014D  .
-r v3 := psp
-goto :eof
-g

Program terminated normally (0300)
-dm
PSP: 3976
02B9 4D 0008 000D     208 B SD
02C7 4D 2975 000F     240 B TESTBIGC
02D7 4D 02D8 269C   154 KiB LDEBUG
2974 4D 2975 1000  64.0 KiB TESTBIGC
3975 4D 3976 1000  64.0 KiB DEBUGGEE
4976 5A 2975 5688   346 KiB TESTBIGC
9FFF 4D 0008 1000  64.0 KiB SC
B000 4D C151 0100   4.0 KiB COMMAND
B101 4D B102 0019     400 B SEEKEXT
B11B 4D 0000 06E3  27.5 KiB
B7FF 4D 0008 0800  32.0 KiB SC
C000 4D 0008 014F   5.2 KiB SD
C150 4D C151 00A9   2.6 KiB COMMAND
C1FA 4D 0000 1E04   120 KiB
DFFF 4D 0008 1000  64.0 KiB SC
F000 4D 0000 096E  37.7 KiB
F96F 5A 0000 0090   2.2 KiB
-l
-dm
PSP: 3986
02B9 4D 0008 000D     208 B SD
02C7 4D 2975 000F     240 B TESTBIGC
02D7 4D 02D8 269C   154 KiB LDEBUG
2974 4D 2975 1000  64.0 KiB TESTBIGC
3975 4D 3986 000F     240 B TESTBIGC
3985 4D 3986 0FF0  63.7 KiB TESTBIGC
-q
dos mem corrupt, first_mcb=02b9
prev 3985:0000|4d 86 39 f0 0f 00 00 00 54 45 53 54 42 49 47 43 Må9≡....TESTBIGC
notMZ4976:0000|cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• 

PANIC: MCB chain corrupted
System halted

  4. ecm-pushbx commented on Jun 3, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 3, 2025
    ContributorAuthor

    Flat-format executables sized >= FF00h bytes are rejected by the MS-DOS v4.01 and lDOS executable loaders. So the problem might be restricted to pathological cases of executable sizes. However, the other condition of this (precise free block size) is not purely pathological, it can occur when loading to UMBs using LOADHIGH.

    Using the value r v5 := FEFF will make the test succeed to load the executable the first time on both FreeDOS and lDOS kernels. It still ends up with FreeDOS corrupting its trailing MCB when continuing the test. So it isn't entirely pathological: The largest valid flat-format .COM executable for MS-DOS v4.01 can elicit this bug from the FreeDOS kernel.

  5. ecm-pushbx commented on Jun 3, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 3, 2025
    ContributorAuthor

    The largest valid flat-format .COM executable for MS-DOS v4.01 can elicit this bug from the FreeDOS kernel.

    This appears to be because the overlapping memory areas are larger than 1 paragraph. Here's another test run with r v5 := FEFF depicting how far the memory corruption reaches:

    
Program terminated normally (0300)
-dm
PSP: 3976
02B9 4D 0008 000D     208 B SD
02C7 4D 2975 000F     240 B TESTBIGC
02D7 4D 02D8 269C   154 KiB LDEBUG
2974 4D 2975 1000  64.0 KiB TESTBIGC
3975 4D 3976 1000  64.0 KiB DEBUGGEE
4976 5A 2975 5688   346 KiB TESTBIGC
9FFF 4D 0008 1000  64.0 KiB SC
B000 4D C151 0100   4.0 KiB COMMAND
B101 4D B102 0019     400 B SEEKEXT
B11B 4D 0000 06E3  27.5 KiB
B7FF 4D 0008 0800  32.0 KiB SC
C000 4D 0008 014F   5.2 KiB SD
C150 4D C151 00A9   2.6 KiB COMMAND
C1FA 4D 0000 1E04   120 KiB
DFFF 4D 0008 1000  64.0 KiB SC
F000 4D 0000 096E  37.7 KiB
F96F 5A 0000 0090   2.2 KiB
-d 4976:0
4976:0000  5A 75 29 88 56 00 00 00-00 00 00 00 00 00 00 00 Zu).V...........
4976:0010  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0020  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0030  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0040  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0050  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0060  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0070  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
-l
-d 4976:0
4976:0000  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0010  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0020  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0030  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0040  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0050  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0060  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0070  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
-dm
PSP: 3986
02B9 4D 0008 000D     208 B SD
02C7 4D 2975 000F     240 B TESTBIGC
02D7 4D 02D8 269C   154 KiB LDEBUG
2974 4D 2975 1000  64.0 KiB TESTBIGC
3975 4D 3986 000F     240 B TESTBIGC
3985 4D 3986 0FF0  63.7 KiB TESTBIGC
-h 3986+FF0
4976  decimal: 18806
-d 4976:0
4976:0000  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0010  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0020  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0030  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0040  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0050  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0060  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0070  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
-
4976:0080  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:0090  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00A0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00B0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00C0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00D0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00E0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC CC ................
4976:00F0  CC CC CC CC CC CC CC CC-CC CC CC CC CC CC CC 00 ................
-
4976:0100  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0110  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0120  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0130  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0140  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0150  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0160  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
4976:0170  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ................
-

  6. ecm-pushbx commented on Jun 3, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 3, 2025
    ContributorAuthor

    I think that this should also round up the file size using a + 15 byte displacement somewhere in there. I feel like this can be exploited to cause MCB corruption as well, but I'm too lazy to create that test case right now:

    kernel/kernel/task.c

    Lines 470 to 473 in b8ef68f

    ULONG com_size_long = SftGetFsize(fd);
    /* maximally 64k - 256 bytes stack -
    256 bytes psp */
    com_size = ((UWORD)min(com_size_long, 0xfe00u) >> 4) + 0x10;

  7. boeckmann commented on Jun 3, 2025

    @boeckmann
    boeckmann
    on Jun 3, 2025
    Contributor

    I think that this should also round up the file size using a + 15 byte displacement somewhere in there. I feel like this can be exploited to cause MCB corruption as well, but I'm too lazy to create that test case right now:

    kernel/kernel/task.c

    Lines 470 to 473 in b8ef68f
    ULONG com_size_long = SftGetFsize(fd);
    /* maximally 64k - 256 bytes stack -
    256 bytes psp */
    com_size = ((UWORD)min(com_size_long, 0xfe00u) >> 4) + 0x10;

    That is exactly the code position I also questioned a long time ago if there should be a +15 added to com_size_long, as it currently rounds DOWN by the shift. Too bad I do not find where I posted this...

  8. boeckmann commented on Jun 3, 2025

    @boeckmann
    boeckmann
    on Jun 3, 2025
    Contributor

    Too bad I do not find where I posted this...

    Found it :) #195 (comment)

  9. boeckmann commented on Jun 4, 2025

    @boeckmann
    boeckmann
    on Jun 4, 2025
    Contributor

    Reading 0xff00U bytes needs an allocation of 0x10000 bytes including the PSP. But the code allocates 0xff00U bytes at max. I guess therefore 256 bytes will be overwritten.

    I changed the kernel source accordingly. I do not get a MCB chain corruption anymore. But under lDebug the l command fails with an out-of-memory error after the generated code of the testbigc.sld was run via g. Is this expected behaviour (still digging through the .sld file)?

  10. boeckmann commented on Jun 4, 2025

    @boeckmann
    boeckmann
    on Jun 4, 2025
    Contributor

    This is what I changed in task.c:

    diff --git a/kernel/task.c b/kernel/task.c
index 8ddb6e3..d3b6511 100644
--- a/kernel/task.c
+++ b/kernel/task.c
@@ -465,12 +465,11 @@ COUNT DosComLoader(BYTE FAR * namep, exec_blk * exp, COUNT mode, COUNT fd)
   UWORD env, asize = 0;
   
   {
-    UWORD com_size;
+    UWORD alloc_size;
     {
       ULONG com_size_long = SftGetFsize(fd);
-      /* maximally 64k - 256 bytes stack -
-         256 bytes psp */
-      com_size = ((UWORD)min(com_size_long, 0xfe00u) >> 4) + 0x10;
+      /* maximally 64k inclusive 256 bytes psp, rounded up to paragraph */
+      alloc_size = (UWORD)(min(com_size_long + sizeof(psp) + 15, 0x10000ul) >> 4);
     }
 
     if ((mode & 0x7f) != OVERLAY)
@@ -494,7 +493,7 @@ COUNT DosComLoader(BYTE FAR * namep, exec_blk * exp, COUNT mode, COUNT fd)
       /* yes, see RBIL, int21/ah=48 -- Bart */
 
       if (rc == SUCCESS)
-        rc = ExecMemLargest(&asize, com_size);
+        rc = ExecMemLargest(&asize, alloc_size);
       
       if (rc == SUCCESS)
         /* Allocate our memory and pass back any errors         */
@@ -513,7 +512,7 @@ COUNT DosComLoader(BYTE FAR * namep, exec_blk * exp, COUNT mode, COUNT fd)
       if (rc != SUCCESS)
         return rc;
 
-      ++mem;
+      ++mem;  /* skip MCB paragraph */
     }
     else
       mem = exp->load.load_seg;

  11. ecm-pushbx commented on Jun 4, 2025

    @ecm-pushbx
    ecm-pushbx
    on Jun 4, 2025
    ContributorAuthor

    Reading 0xff00U bytes needs an allocation of 0x10000 bytes including the PSP. But the code allocates 0xff00U bytes at max. I guess therefore 256 bytes will be overwritten.

    Indeed.

    I changed the kernel source accordingly. I do not get a MCB chain corruption anymore. But under lDebug the l command fails with an out-of-memory error after the generated code of the testbigc.sld was run via g. Is this expected behaviour (still digging through the .sld file)?

    Yes, it is.

  12. ecm-pushbx commented on Dec 5, 2025

    @ecm-pushbx
    ecm-pushbx
    on Dec 5, 2025
    ContributorAuthor

    No fix seems to have been committed yet. I may study your patch sometime again.

  13. ecm-pushbx commented on Dec 5, 2025

    @ecm-pushbx
    ecm-pushbx
    on Dec 5, 2025
    ContributorAuthor
    r byte [cs:ip + 1] := -1			; workaround
l
if (rc) then goto :eof				; this doesn't work really
if (byte [cs:ip + 1] != CC) then goto :eof	; workaround

    Regarding this bit of the Script for lDebug presented here, I just added a changeset that fixes the RC setting so that the L command will set the RC variable on errors. This will be included in today's build that is going to be available at https://pushbx.org/ecm/download/old/ldebug/20251205.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions