Skip to content

GetModuleFileName is passed a byte count where the API expects a character count #101

Description

@bero

GetModuleFileNameAsString in Source/Common/Support/BoldUtils.pas passes SizeOf(Buffer) to the Win32 GetModuleFileName, whose nSize parameter is documented as a count of TCHARs, not bytes. The same mistake appears a second time in Source/Common/IDE/BoldAbout.pas. Found while debugging the ASP example's ISAPI hosting, where ModuleDirectory resolves the database path through this function.

Symptom

Latent — no reported failure, because no current caller is known to reach it. A module path longer than 261 characters writes past the end of a stack buffer.

Root cause

BoldUtils.GetModuleFileNameAsString, lines 146-157:

function GetModuleFileNameAsString(IncludePath: Boolean): string;
var
 Buffer: array[0..261] of Char;
 ModuleName: string;
begin
  SetString(ModuleName, Buffer, Windows.GetModuleFileName(HInstance,
        Buffer, SizeOf(Buffer)));
  1. Buffer holds 262 Char. Char is WideChar, so the buffer is 524 bytes.
  2. SizeOf(Buffer) is therefore 524.
  3. nSize is documented as "the size of the lpFilename buffer, in TCHARs". Windows is thus told the buffer can take 524 wide characters — 1048 bytes — when it can take 262.
  4. A module path longer than 261 characters overruns the stack buffer by up to 524 bytes. Reachable on Windows 10 1607+ with LongPathsEnabled, or for a module loaded from a deeply nested path.

Secondary effect: GetModuleFileName signals truncation by returning nSize. With the inflated value that comparison cannot match the buffer's real capacity, so truncation goes undetected.

BoldAbout.pas:198 repeats it against a 261-element buffer:

  FileName: array [0..260] of Char;
  ...
  if GetModuleFileName(hInstance, FileName, SizeOf(FileName)) > 0 then

Fix

  • BoldUtils.GetModuleFileNameAsString — pass Length(Buffer) rather than SizeOf(Buffer). Optionally grow the buffer and retry while the return value equals the buffer size, so paths beyond MAX_PATH resolve instead of silently truncating.
  • BoldAbout — the same substitution at line 198.

Files affected

  • Source/Common/Support/BoldUtils.pas
  • Source/Common/IDE/BoldAbout.pas

Notes

  • Present since 92196d9 (2021-04-26, "Merge of AttracsBold"). It predates the open-source history here and is not a regression.
  • Callers in Source/: BoldPropagatorMainForm.pas:304 and BoldPropagatorServer.pas:164. The ASP, HTTPPMapper, XML and XML2 examples also call it.
  • The HInstance argument is correct — inside a DLL it returns the DLL's own path, which is what the ASP example relies on. Only the size argument is wrong.

Testing

  • Both call sites located; buffer declarations confirmed as array[0..261] of Char and array[0..260] of Char
  • Test covering a module path longer than 261 characters
  • Full suite green after the change

Activity

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions