mORMot Open Source

tbo

Member

Registered: 2015-04-20

Posts: 353

Function SafeFileName() should be extended

The name of the function promises more than it does. In conjunction with inconsistencies in Delphi, it would be beneficial if the function could do more. In Delphi, functions that should do the same thing behave differently. Here is an example:

fileName := 'f:\test/test/../data/test.txt';
fn := ExtractFileName(fileName);  // Result => 'test/test/../data/test.txt'
fn := TPath.GetFileName(fileName);  // Result => 'test.txt'

If you have parameter values from outside, this must be taken into account. Couldn't a function, illustrated with the following pseudocode, be useful?

type
  TFileNameValidation = (fnvDirName, fnvFileName);
  TFileNameValidations = set of TFileNameValidation;

function SafeFileName(const pmcFileName: TFileName; pmValidationParts: TFileNameValidations; pmCheckedFileName: PFileName = Nil): Boolean;
var
  dirName, fileName: TFileName;
begin
  if (pmcFileName = '') or (pmValidationParts = []) then Exit(False);

  Result := True;
  if fnvDirName in pmValidationParts then
  begin
    dirName := TPath.GetDirectoryName(pmcFileName);
    Result := Result and TPath.HasValidPathChars(dirName, False);
  end;

  if Result
    and (fnvFileName in pmValidationParts) then
  begin
    fileName := TPath.GetFileName(pmcFileName);
    Result := Result and TPath.HasValidFileNameChars(fileName, False);
  end;

  if Result
    and (pmCheckedFileName <> Nil) then
  begin
    if (dirName <> '') and (fileName <> '') then
      pmCheckedFileName^ := TPath.Combine(dirName, fileName, False)
    else if fileName <> '' then
      pmCheckedFileName^ :=  fileName
    else
      pmCheckedFileName^ :=  dirName;
  end;
end;

With best regards
Thomas

Last edited by tbo (2022-07-03 21:30:08)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: Function SafeFileName() should be extended

I don't understand which promise you expect from the description: this function searches for some patterns in the file name, like '..' which are unsafe to be used in concatenation with a root folder name, if you want to be within this folder name.

tbo

Member

Registered: 2015-04-20

Posts: 353

Re: Function SafeFileName() should be extended

I don't understand which promise you expect from the description:

My next article is titled: "Introduction to Method based Services - Server and Client". The example for the article is a small up and download portal. In the process, I noticed that securing the functions requires more effort than I had first thought. The reason is that the combination of user input and the above-mentioned behavior of some functions opens unexpected access possibilities. The first version I wrote looked perfectly safe, but it was not. The second version was then a little bit paranoid. The proposed version may be overcautious, but it would meet my expectations for a function named SafeFileName. Or concretely, a function SaveFile() should be perfectly safe with the following source code:

if (pmCtxt.Method = mPUT)
  and GetSessionUserDirName(pmCtxt, dirName)
  and UrlDecodeNeedParameters(pmCtxt.Parameters, Param.ImageName) then
begin
  Utf8ToFileName(pmCtxt.InputUtf8[Param.ImageName], imageName);
  if SafeFileName(imageName, [fnvFileName], @imageName) then ...

Personally, I would never solve the task this way. I would build a virtual file system with the ORM. But that would be too complex for an example. So I have no need for this function. But the argument against using mORMot is often the complexity and that developers don't trust themselves to use it safely.

With best regards
Thomas

Last edited by tbo (2022-07-04 11:59:24)