BUG because of the successful upload of hfs.diff.tpl

[Mars]

 function complyUploadFilter():boolean;    //mod by mars  uploadFailed was not initialized with DIFF_TPL_FILE
  var
    s: string;
  begin
  result:=FALSE;
  if ansiContainsText(data.uploadSrc, DIFF_TPL_FILE) then
    begin
    data.uploadFailed:='File name forbidden';
    exit;
    end;
  if f.isTemp() then s:=f.parent.uploadFilterMask
  else s:=f.uploadFilterMask;
  if s = '' then s:='\hfs.*;index.htm*;default.htm*;descript.ion;*.md5'; // the user can disable this default filter by inputing * as mask
  result:=fileMatch(s, data.uploadSrc);
  if result then exit;
  data.uploadFailed:='File name or extension forbidden';
  end; // complyUploadFilter

[rejetto]

hfs.diff.tpl should be matched by hfs.* in the default mask.
maybe you specified a different mask for that folder?

[Mars]

hfs.diff.tpl should be matched by hfs.* in the default mask.
maybe you specified a different mask for that folder?

original code of hfs is

result:=FALSE;
  if ansiContainsText(data.uploadSrc, DIFF_TPL_FILE) then exit;
  if f.isTemp() then s:=f.parent.uploadFilterMask
 
in this case all files matched   *hfs.diff.tpl* are are exread by the upload, BUT  the obtained answer is: transferred file as in the image before.png

make test with upload mask file as *.*
and upload a hfs.diff.tpl file to the real folder

what append for you??

forbid all upload of files as hfs.diff.tpl is a good security, don't delete it , just inform the user that it is a secure file of hfs;)

[rejetto]

yes, the diff tpl is much more dangerous than other masked files.

i changed "ansiContainsText" in "sameText". silly error.
i changed it to this

Code: [Select]

  function complyUploadFilter():boolean;

    function getMask():string;
    begin
    if f.isTemp() then result:=f.parent.uploadFilterMask
    else result:=f.uploadFilterMask;
    if result = '' then
      result:='\hfs.*;index.htm*;default.htm*;descript.ion;*.md5'; // the user can disable this default filter by inputing * as mask
    end;

  begin
  result:=not sameText(data.uploadSrc, DIFF_TPL_FILE)
    and fileMatch(getMask(), data.uploadSrc);
  if not result then
    data.uploadFailed:='File name or extension forbidden';
  end; // complyUploadFilter


[Mars]

In some templates is offered to the user the possibility to rename a file as for example xxx.txt  to hfs.diff.tpl


  

 if name = 'rename' then
      begin
     if ( filename(p) <> DIFF_TPL_FILE) and (filename(par(1)) <> DIFF_TPL_FILE) then
        spaceIf(renameFile(p, par(1)))
      else result:='';
      if (result > '') and not stopOnMacroRename then // should we stop recursion?
        try
          // by default, we'll stop after first stacked [on macro rename], but recursive=1 will remove this limit
          stopOnMacroRename:=isFalse(par('recursive'));
          runEventScript('on macro rename', toSA(['%old-name%',p,'%new-name%',par(1)]));
        finally
          stopOnMacroRename:=FALSE;
          end;
      end;

then it is possible to upload one file with our template and rename it to hfs.diff.tpl.

Silentpliz was my victim to make this test, I was a gentile not to put him a template with one macro exec in the main section



same bug security with macros:

    if name = 'move' then
      spaceIf(movefile(p, par(1)));

    if name = 'copy' then
      spaceIf(copyfile(p, par(1)));

    if (name = 'save') or (name = 'append') then
      save();

    if name = 'delete' then
      if isTrue(par('bin',TRUE,'1')) then
        spaceIf(moveToBin(p, isTrue(par('forced'))))
      else
        spaceIf(deltree(p));

 

[rejetto]

very important point.

my opinion is that  we should focus on the script-side security (a safe script instead of a safe scripting engine).
i give two reasons for this:

1. total prevention is hard or impossible to achieve: i can use {.save.} to create a batch file with "ren my hfs.diff.tpl", and the {.exec.}
even if you would want to detect the string, i can split it in two parts, and use {.append.}
so, on this side, it seems to be a battle already lost.

2. being able to manipulate hfs.diff.tpl can be an interesting opportunity for useful features in scripts.


that's why i think this way, the problem is giving such power to the wrong user.
working on this side, we may try to mark unsafe scripts (on the forum, or in a repository), or alerting about the intended audiance (admin, occasional surfers, etc), or giving commands to ease the making of safe scripts.

[SilentPliz]

i changed "ansiContainsText" in "sameText". silly error.

Choosing Semtex would have been more explosive !

I go outside ...