If the command line is long, it looks like code might get overwritten

[dcoshea]

While looking at the handling of the command line because I was considering adding a new parameter, I noticed that toward the end of the code section starting at the keepumbs2: label, and also at the following parmsgiven: label, there is code which potentially appends to the command-line tail in the Program Segment Prefix, and it seems to me that if the command line was long enough it could overrun that buffer.

Specifically, starting at:

devload/devload.asm

Line 415 in 3fd3dd6

; Find end of command line in BX.

                mov     bl,byte [80h]
                add     bx,81h

If the PSP is valid, the byte at offset 80h could potentially be 7fh (RBIL) - i.e. the entire size of the command-line tail buffer - so BX could be 100h at this point, which would mean that it is pointing at the start of the code (Main0).

However FreeCOM recently had a bug where it could set the length to greater than 7fh (FDOS/freecom#51), and I don't see any evidence that this code validates the length anywhere, so the situation could be worse with an invalid PSP.

The following code compares the offset past the end of the command-line tail in BX to the offset past the end of the driver filename which was previously stored in DI:

        ; Compare them.

                cmp     bx,di
                ja      parmsgiven

        ; If they are the same, no parameters were given, so add a space.

                mov     byte [di],' '
                inc     bx

The above code then conditionally appends a space to the command-line tail in the PSP,

        ; Append CrLf to line.

parmsgiven:     mov     word [bx],0A0Dh

and the above unconditionally appends a carriage return and line feed.

It seems unlikely that in the case where there are no parameters the command line would be long enough to trigger this issue unless a long path to the driver file was specified, but that's not impossible.

Even in that worst case where three characters are appended (space, carriage return, line feed), the first instruction at Main0 is a near JMP, which is 3 bytes long, and it's presumably never executed again so it probably doesn't matter if it gets overwritten.

In the case of a PSP with an excessively long command-line tail length byte (greater than 7fh), it would seem to me that code at relocated: could get overwritten, and presumably that would actually be bad.

[PerditionC]

It looks like there are 2 places where it doesn't validate command line length, when parsing for devload options and as you describe above. I will update the code this week with a check, and see if it makes sense to actually handle the long command lines. Perhaps update devload to support them itself but not passed to loaded driver as long?

[dcoshea]

Perhaps update devload to support them itself but not passed to loaded driver as long?

That sounds reasonable and much easier than doing the work to actually support passing everything through to the loaded driver. It seems unlikely that anyone is going to be bothered by the command-line length limit being a few characters shorter than it could be.