View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005809JEDI Code LibraryJclBorlandToolspublic2012-02-25 06:322012-10-28 16:54
ReporterdeboseAssigned Tooutchy 
PrioritynormalSeverityblockReproducibilityalways
Status resolvedResolutionfixed 
Product VersionVersion 2.4 
Target VersionFixed in VersionVersion 2.4 
Summary0005809: Unsafe code in JclMsBuild.pas
DescriptionJclMsBuild.pas, line 1015, function TJclMsBuildParser.EvaluateString.
There is code to replace EnvVariable with it's value (f.e. $(BDS) with c:\Program files....). Code:
  StrReplace(Result,
    Copy(Result, Start, Position - Start + 1), // $(PropertyName)
    PropertyValue,
    [rfReplaceAll])

But, there is possible situation, when PropertyValue will be equal to EmptyStr. In this case, these variables will be simply deleted. This error may lead to getting wrong paths for LibrarySearchPath.

I suggest to add check before executing StrReplace:
if PropertyValue<>EmptyStr then

p.s. I ran into this error when ran my .exe outside Delphi, and it's only occures for me when I try to get LibrarySearchPath for Delphi 2010. See Additional information.
Additional InformationExample project.
Delphi 2010 should be installed. This program should be compiled and run from outside the IDE. Variables BDS, BDSCOMMONDIR, .. should be undefined for this project's session.


program Test;

{$APPTYPE CONSOLE}

uses
  JclIDEUtils,
  SysUtils;

var
  tmpIs: TJclBorRADToolInstallations;
  tmpI: TJclBorRADToolInstallation;
  s: string;
begin
  try
    tmpIs := TJclBorRADToolInstallations.Create;
    // get Delphi 2010
    tmpI := tmpIs.BDSInstallationFromVersion[7];
    Writeln('LibrarySearchPath: '+tmpI.LibrarySearchPath[bpWin32]);
    Writeln;
    Writeln('RawLibrarySearchPath: '+tmpI.RawLibrarySearchPath[bpWin32]);
    Writeln;

    Readln;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.
TagsDelphi 2010, Jcl, JclMsBuild, MsBuild
Fixed in GIT commit
Fixed in SVN revision3854
IDE versionDelphi/C++Builder 2010

Activities

outchy

2012-02-25 21:12

administrator   ~0019553

This should be fixed in revision 3749.
But the function JclCompilerUtils.TJclDCC32.AddDProjOptions still does not take care of RsVars.bat.

outchy

2012-03-05 22:10

administrator   ~0019649

Revision 3762 fixes TJclDCC32.AddDProjOptions.

debose

2012-05-27 01:20

reporter   ~0019783

Code is still unsafe. There are 2 issues:
1) issue 0000001: EvaluateString method ripps off unknown variables
Example:
program Project1;
{$APPTYPE CONSOLE}
uses
  SysUtils,
  JclMsBuild;
var
  tmpMsb: TJclMsBuildParser;
  s: string;
begin
  try
    WriteLn('issue 0000001: EvaluateString method ripps off unknown variables');
    tmpMsb := TJclMsBuildParser.Create('AnyExistingDprojFile.Dproj');
    try
      //tmpMsb.Init; // it doesn't matter if call Init or not

      //
      s := '$(TEST_NON_EXISTING_VAR)';
      WriteLn('EvaluateString for '+s);

      // we have to call tmpMsb.Init to avoid AV calling tmpMsb.EvaluateString(s)
      s := tmpMsb.EvaluateString(s);
      WriteLn(Format('Result = "%s"',[s]));
      WriteLn('We lost him =(');
      Readln;
    finally
      tmpMsb.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

debose

2012-05-27 01:21

reporter   ~0019784

2) Issue #2: TJclMsBuildParser Created with default constructor Create, leads to Access Violations.

program Project1;
{$APPTYPE CONSOLE}
uses
  SysUtils,
  JclMsBuild;
var
  tmpMsb: TJclMsBuildParser;
  s: string;
begin
  try
    WriteLn('issue #2: TJclMsBuildParser Created with default constructor Create, leads to Access Violations');
    tmpMsb := TJclMsBuildParser.Create();
    try
      tmpMsb.Init; // AV here
    finally
      tmpMsb.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

debose

2012-05-27 15:54

reporter   ~0019785

Issue 3: Endless loop in EvaluateString for recursive property values

program Project1;
{$APPTYPE CONSOLE}
uses
  SysUtils,
  JclMsBuild;
var
  tmpMsb: TJclMsBuildParser;
  s: string;
begin
  try
    WriteLn('issue 3: Endless loop in EvaluateString for recursive property values');
    tmpMsb := TJclMsBuildParser.Create('AnyExistingDprojFile.Dproj');
    try
      tmpMsb.Properties.Values['TEST']:='VALUE;$(TEST)';
      s := tmpMsb.EvaluateString('$(TEST)'); // endless loop here
    finally
      tmpMsb.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

debose

2012-05-27 16:00

reporter   ~0019786

I met all these issues when I tried to parse DefProject.bdsproj from Delphi XE with JclMsBuild (I converted DefProject.bdsproj Tags to make XML structure compatible with .Dproj-format which can be handled by TJclMsBuildParser):
I tried to fix EvaluateString myself, but did not succeeded. So, I will move away from using JclMsBuild, which means, that reported issues are not important for me anymore.

debose

2012-05-27 17:35

reporter   ~0019787

About "issue 1: EvaluateString method ripps off unknown variables". I look deeper in sources, and I found that "ripping off" is the feature that provides correct Condition parsing. So I might be wrong about calling it a bug.

Additionally, I suggest you to refactor TJclMsBuildParser making it more flexible. F.e., replace code
PropertyValue := Properties.Values[PropertyName]
with
PropertyValue := GetPropertyValue(PropertyName);

+ create "protected" function
function GetPropertyValue(const aPropName: string): string; virtual;
begin
  result := Properties.Values[PropertyName]
end;

It will allow to create TJclMsBuildParser descendant and override that method to provide variable value if it was not found by TJclMsBuildParser itself. Also I can handle "Issue 3: Endless loop in EvaluateString for recursive property values" in descendant.

outchy

2012-08-30 15:50

administrator   ~0020135

Good idea, this is committed in revision 3854.

Issue History

Date Modified Username Field Change
2012-02-25 06:32 debose New Issue
2012-02-25 06:32 debose IDE version => Delphi/C++Builder 2010
2012-02-25 06:33 debose Tag Attached: Jcl
2012-02-25 06:33 debose Tag Attached: Delphi 2010
2012-02-25 06:33 debose Tag Attached: JclMsBuild
2012-02-25 06:33 debose Tag Attached: MsBuild
2012-02-25 21:12 outchy Note Added: 0019553
2012-03-05 22:10 outchy Note Added: 0019649
2012-03-05 22:10 outchy Assigned To => outchy
2012-03-05 22:10 outchy Status new => feedback
2012-04-08 19:01 outchy Fixed in revision => 3762
2012-04-08 19:01 outchy Status feedback => resolved
2012-04-08 19:01 outchy Fixed in Version => Version 2.4 (Subversion repository/Daily zips)
2012-04-08 19:01 outchy Resolution open => fixed
2012-05-27 01:20 debose Note Added: 0019783
2012-05-27 01:20 debose Status resolved => feedback
2012-05-27 01:20 debose Resolution fixed => reopened
2012-05-27 01:21 debose Note Added: 0019784
2012-05-27 15:54 debose Note Added: 0019785
2012-05-27 16:00 debose Note Added: 0019786
2012-05-27 17:35 debose Note Added: 0019787
2012-08-30 15:50 outchy Note Added: 0020135
2012-10-28 16:54 outchy Fixed in revision 3762 => 3854
2012-10-28 16:54 outchy Status feedback => resolved
2012-10-28 16:54 outchy Resolution reopened => fixed