ALM buffer overrun (MCR10002)

[Eric Swenson]

ALM 8.14 has a buffer overrun bug related to the pathname length of the source file being assembled.
"alm_symtab_" uses "prname_" to place the source file path in the listing file.
"prnam_" uses the buffer oulst to return the formatted string.
"oulst" is declared in "prnam_.pl1" and "als_.pl1" as

dcl       1 eb_data_$oulst external, 2 oulst char(68) aligned;

and is defined in "eb_data_.alm" as

    segdef  oulst
oulst:  bss oulst,17

A sufficently long source file path will overrun the buffer and damage ALM's data structures leading to a variety of issues depending on the path length and contents. Out-of-bounds and phase errors have been observed.

As a test, the ALM source was modifed, changing the length of "oulst" to "char (168)", the "bss oulst" to ",42" and "srclen = 68" to "168" in the files "alm_.pl1", "eb_data.alm" and "prnam_.pl1".

After the changed, my test script ran without error.

The source code needs to be reviewed to determine to correct size of "outst", taking into account the Multics maximum path length and "prnam_" overhead, and that all relevant code is fixed.

[Eric Swenson]

Ticket moved here from ​https://sourceforge.net/p/dps8m/tickets/89/

[Eric Swenson]

AG-91, pg 3-1: Entry name max. length is 32 characters/

AG-91, pg 3-2 'A maximum of 16 levels of directories is allowed from the root to the final entryname.'

16 levels is 16 * (32 + 1 [for the '>]) + 32 [the entryname] +1 [for the '>']

16 * 33 + 33

17 * 33

561 characters.

[Eric Swenson]

MCR10002 implemented Charles' fix to ALM for this issue. However, there was discussion as to whether this was the correct fix, so leaving this ticket open for further discussion.

[Eric Swenson]

The only issue I have is the details of the implementation; should the manifest constant 168 be replaced with the Multics equivilent of the "MAX_PATH_LEN"

[Eric Swenson]

Reopening ticket so we can discuss a better fix (if any).

[Eric Swenson]

​http://swenson.org/multics_wiki/downloads/doc/MCR10002.txt addresses this ticket.

[Eric Swenson]

While it is true that the hierarchy depth can be at most 16, the absolute pathname of any entry can be a maximum of 168 characters. So the original fix, to allow up to 680 characters is overkill. The char array should use 168 (or if there is a manifest constant for this, that should be sued). The number of words (originally 17, updated to 170 by the previous fix), should be 168/4=42.

[Eric Swenson]

I see no manifest constants for 168 in >ldd>incl>.pl1 or >ldd>incl>.alm that would be suitable for use in the three segments that need to be updated here. I see hundreds of references to char(168) in the source tree, so I see no reason why we need to be hard-nosed about the of a manifest constant here. I think we should do like other code does -- declare the char array size 168 (with possibly a comment saying that this is the maximum size of a pathname). There is one source file where the number of words consumed by 168 characters (42) needs to be specified. A comment here indicating that 168/4 is 42 would be appropriate, and a reference, again, to where 168 comes from.

Comments?

[Gary Dixon]

VENT: The code in bound_alm_.s.archive is atrocious.

Having gotten that off my chest...

The maximum length of a pathname seems irrelevant to any discussion of the proper length of eb_data_$oulst.oulst variable. The main routine I've found for filling that variable is prnam_.alm. I have found no code calling prnam_ with a pathname to put in the output buffer.

prnam_ copies either 1 or 2 acc strings into the oulst variable. An acc string is a word-aligned character string that begins with a length character (fixed bin(9) unsigned) specifying the total number of characters in the acc string (not including the acc length character). Thus, an acc string may be up to 512 characters long (including the acc length character). The prnam_$prnam2 entry point copies into the oulst variable two such acc strings separated by "|" and ending with a NL character. The acc length character is not copied with the second string.

Thus, the maximum length needed for this output line buffer is: 512 + 511 + 2 (for "|" and NL chars) = 1025

I doubt that the actual acc strings it is called upon to copy are anywhere near that length. The acc strings I've seen appear to be text strings describing elements of a symbolic link. These could be <segname>|<entrypoint_name>. While a <segname> is limited to 32 characters, an <entrypoint_name> can be up to 256 chars long. Thus, the max length of 289 is needed for <segname>|<entrypoint_name>.

I haven't found the code in bound_alm_.s.archive that actually outputs the data placed in the oulst variable. How does it determine the actual length of characters currently in that buffer?

It is also apparent that attempting an ALM assembly in one process, interrupting that assembly to do a second assembly, then resuming the first assembly is frought with problems. Since eb_data_ holds its impure data in the linkage section of eb_data, and since that linkage section is shared by ALL invocations of alm in that process, the 2nd assembly could step on the data deposited any in eb_data_ by the interrupted first assembly operation. But that is a different and much larger issue.

In short, it is likely that the original value of 68 characters was too short. It is likely that 168 might be too short as well. It is possible but unlikely that the current length of oulst (680) may be too short. But 680 is a reasonable guess until someone finds a need to make it smaller because too much data is being used in the process' combined linkage area by bound_alm_; or larger, because a someone tries to compile a program that has very long entrypoint_names, etc.

[Eric Swenson]

While I agree that the value of 680 is probably "good enough", it is a "random" number. It is 10*68, where 68 was the original value in MR12.5. I think it was picked out of a hat, really, given that the calculations above are incorrect (pathnames cannot be as long as the ticket stays, they are limited to 168 characters).

And while I agree that it is quite possible that there are "other" strings emitted to the oulst buffer besides pathnames (indeed, most are short symbol names), I did run ALM through a test where I kept track of the largest number of words copied to the oulst buffer. After assembling about 50 assembly source files with the -list option enabled, the ONLY time I could see a large number (bigger, actually than the original 68-character/17-word limit) was when the pathname of the source file was emitted to the .list segment. So while your argument, Gary, may well be correct -- that it is theoretically possible for longer symbols to be copied to this buffer, in practice I could never get more than 168 characters to get copied. My test cases included some pathnames that were 168 characters long, and it was only THOSE that resulted in "large" (41-42 word) writes.

So despite the fact that you believe that pathnames are irrelevant, in practice, it was ONLY the pathnames that exceeded the original 68-character buffer. Of course, the current 680-character buffer is more than enough.

So I agree what we can simply close this ticket -- unless we can find a test case where a 680-character/170-word buffer is not big enough.