Help Understanding SQL Parameters Utilization

[Gary Mugford 12]

I have come to understand that SQL code injection is a bad, bad, BAD thing for we programmers to do. BUT I've come up with two questions regarding switching to parameterization in my code. I'm using Delphi 7 and BDE as my bog standard and have XE7 so if there are differences in your answer(s) between the two, I'd appreciate the extra paragraph. 

 

My first question is the regular use of the same 'variable' name in the SQL lines in the query object. IF I include:

DateExpected between :eDF and :eDT

Do I run any risks using EXACTLY the same variables, albeit maintaining dataType, possibly applied to a different field in a different query, so that I can standardize on the lines of codes dealing with the Parameters that I use in the code before issuing the .Active := true or ExecSQL lines? I am TRYING to remain consistent that eDF is a DateTime variable, but what if I do something stupid like use :A for the variable and make it a string variable in one query and an integer variable in a second query somewhere in proximity in the code to the first??

 

Next, I have a question where I see no way AROUND using code injection. Say I have a filter I want to apply to the query in the query construction stage. Is that not a case where I NEED injection?? Simple example case in point:

procedure TForm1.Btn1Click(Sender: TObject);
begin
  with s do begin
    active := false;
    Sql.Clear;
    sql.BeginUpdate;
    sql.add('SELECT distinct');
    sql.add('  s.PartNum, s.RAF');
    sql.add('FROM');
    if xZZ.checked
      then sql.add('  "SHIPPING.DB" s, "CATALOG.DB" c')
      else sql.add('  "SHIPPING.DB" s');
    sql.add('WHERE');
    sql.add('  (s.DateDidShip > "' + dateToStr(date-366) +'")'); /// CHANGE to a parameter
    if xZZ.checked then begin
      SQL.add(' and (s.PartNum=c.PartNum) ');
      SQL.add(' and (c.Revision <> "ZZ")');
      end;
    sql.EndUpdate;
    active := true; 
    end; // end sql query for s
  pip.Print;
  ppFooterBand1.visible := false;
  exporttoExcel(pip,'c:\data\Active.XLS', false, true, false);
  ShowMessage('Done! Saved to: c:\data\Active.XLS');
  close;
end;

I see no way to turn this into a set of strings I can call from the original query object. I have a condition (a checkbox on the form) that is used twice, which changes the FROM clause and then the WHERE clause. I CAN and will change the DateDidShip comparison to a parameter. But the other two IF sections?? And remember, this is D7 and BDE. I know this probably is handled easily in newer versions and different databases. But I'm dancing with the same old partner as I have forever. This example might be more easily handled with two SQL.text strings using the dating parameter. But what if there are a half-dozen or even more, switches/filters that are applied to the SQL object's SQL text?? There has to be a complexity line SOMEWHERE, where constructing as above does actually make sense.

 

Any and all guidance appreciated. Updating software is not a currently realistic option. Thanks, GM

 

[FPiette 380]

SQL injection is not something you - as programmer - do, it is something you prevent. It comes from having a SQL query constructed using strings entered by the user. The query you show which is built by code doesn't seems to use data entered by the user. So no problem.

 

If - as an example - the constant "ZZ" must be changed by something coming from the user interface, then use a parameter. You'll have to make the same condition where you'll give a value to the parameter to avoid giving a parameter if the parameter doesn't exists because it is not part of the query because of the condition evaluated to FALSE. Using the same condition around parameter value assignment will solve your problem.

 

[Guest]

SQL Injection is overrated as a threat when using "our stuff". Shure it can happen, but it's mostly on LAMP because "hackers" know MySQL and can figure out how to type something that will break you strings and still execute the query. A really good hacker that knows or guesses at your SQL should, even so, have a bit of work to do in figuring out how to change a select to a delete.

 

OK, so on a more serious note;

 

I very seldom do NOT use parameters. Some stuff is impossible with parameters, often this is why the DAC has "macro" functionality.

 

Using parameters you should be aware of a Querys state a bit more than before. You can Prepare the query. It is at this moment the parameters are resolved to their types and such. So the parameters are handled as sub-objects of the query (application side) and there will be no "mixing them over queries" at all. FQuery1.Params[0] is not the same as FQuery2.Params[0]. No worries.

 

So you write you query depending on RDBMS, DAC and in some cases even flavours of connection... You can re-use the same parameter name in the SQL more than once if the DAC supports it. Sometimes it's even allowed to use parameters in CTE's and in the select clause, for example SQL

"select case when :SHOW_FULLNAME = True then table.full_name else table.shortName end as Name, table.code...". So no, you would not need "injection" just because it's a "filter".

 

Once your query is prepared you can set the parameters and execute/activate. You do not need to unprepare to change the parameter values. Depending on DAC, after changing the parameter values, do Close/Open, RefreshAll or some such (this is what happens when you use TDataSets master-detail functionality, the detail is re-executed for each scroll on the master. So re-executing is faster than changing the SQL and executing (because it has to be prepared anyway)).

 

That is why i use parameters. Efficiency. Let's say you have a REST server with 10 different queries all with parameters w/o need to "parse" the SQL. You can have them prepared, pick the correct Query from a pool, change the parameter, get the data and move on. Much more efficient than creating a new SQL for each request. This scenario works well in non-REST setups too. You can pool connections and/or queries. In my REST servers i pool entire DataModules with lots of queries. Then use either timeouts of DAC functionality to ensure that the RDBMS does not have unnecessary resources hogged. But hold that off until you are comfortable with the prepare stage and parameters.

 

Security is another ballpark, IMHO, if you prefer to parse SQL's (or other stuff like HTTP requests and results) using Delphi string handling i think it's FINE as long as you put your head around what kind of data that can come in and how you should make sure no one can break you processes by entering faulty data and/or making strange requests. THAT should always be a concern even when writing a 20 line console app and it's not just for SQL.

 

So "Do not inject" is a (bad) security advice that lacks a LOT of information and insight.

 

HTH,

 

/D

 

RDBMS = Relational Database Management System, like Oracle, Firebird et. al.

DAC = Data Access Components (BDE, IBX, FireDAC, IBObjects...)

REST = internetty thing, OT

LAMP = Linux, Apache, MySQL and PHP

Edited May 1, 2021 by Guest
Typos

[Guest]

Most times when SQL injection is "found" the LAMP stack just returns an SQL error message. Some setups display the SQL in the browser to the end user. Don not do that.

 

Another benefit with parameters is that you will have a lot less DeQuoteString and a lot less annoying counting of '''' and other escape chars 🙂

[Stano 143]

I can't help but feel sick when I see sql.add ().

• The readability of the code is lousy
• I do not know if this also applies to your case. In general, a parsing of SQL text is performed after each sql.add. It is an unnecessary waste of system resources

First, I compile the entire SQL text according to the user's choices and at once I assign the entire text to SQL. I usually have a separate function for each choice. So I won't get lost in it.

 

I apologise. It happened by mistake.

Edited May 1, 2021 by Stano

[FPiette 380]

5 minutes ago, Stano said:

Nemôžem si pomôcť a je mi zle, keď vidím sql.add ().

This website is in English. Please translate to English before posting.

 

[Guest]

"select distinct s.partnum, s.raf

from shipping s

  left join catalog c on (c.partNum = s.partNum)

where  (s.DateDidShip > :ship_date) and

            (cast(:ZZ as smallint) is null or (c.revision <> 'ZZ'))"

 

if not Query.Prepared then

  Query.Prepare;

Query.Params[0].AsDateTime = Now;

Query.Execute;

DoExportDataSetToHeaven(Query);

 

Should work in Firebird, not tested.

Edited May 1, 2021 by Guest

[Guest]

I agree both with @Stano and @FPiette.

Google Translate identified the text as "Slovak".

The translation was quite legible.

 

Pán Piette hovorí, že pred odoslaním by ste mali napísať text do google translate, aby som to nemusel robiť 🙂

[Gary Mugford 12]

First of all, thanks all for clearing up some of my misunderstandings. Let me clarify my code better than I did in just plopping it into the code brackets.

ZZ is NOT a variable, but a checkbox object CALLED xZZ. And because it might be true or false, the SQL code changes somewhat considerably for such a short piece of code. That is in BDE and thus doesn't get to use things like cast. I assume NexusDB won't have the issue once I transform the megalines of code, using a closer to current IDE like XE7 ... which is what, eight generations old at this point?? In fact, this procedure dates back to 1998. 

 

FPiette: "If - as an example - the constant "ZZ" must be changed by something coming from the user interface, then use a parameter. You'll have to make the same condition where you'll give a value to the parameter to avoid giving a parameter if the parameter doesn't exists because it is not part of the query because of the condition evaluated to FALSE. Using the same condition around parameter value assignment will solve your problem."

 

Because of the ZZ confusion, some of what you say is based on a misapprehension my writing caused. I THINK you were trying to answer my first issue about reusing parameter NAMES. Especially if different dataTypes. As such, I'm NOT sure what you are saying and what I'm asking are the same problem. If you could clarify I would appreciate it.  I'm VERY much trying to use parameters, but it takes time to fix LOTS and LOTS of OLD code with on-the-fly generation that has become my habit. Thanks.

 

Dany, the contained code won't work in BDE. I have been programming simple small internal server apps that began as peer-to-peer apps using Lantastic which began as standalone apps using just Paradox, not even Delphi. Been going almost strong for close to four decades now. I'll probably be retired when my successor takes a look at my code, tosses it almost immediately and starts programming a 21st century interface for the IoT that I have nothing to do with. Thanks for clearing up my confusion about using parameters as a competing concept vs On the Fly SQL Code creation. I really thought the latter was actually code injection. Old dog, new tricks. 

 

Stano, always appreciate anybody spending their time on my behalf. Thanks. 

[FPiette 380]

@Gary MugfordI was talking about the string constant "ZZ" and not the Checkbox named xZZ. I tried to say that yoyr SQL request built with Delphi code is OK and DO NOT suffer of SQL injection risk. I was trying to say that if instead of the constant "ZZ" you want to use a value from a TEdit, then suddenly your code becomes at SQL injection risk and to avoid it you have to use a parametrized SQL request.

[Gary Mugford 12]

29 minutes ago, FPiette said:

@Gary MugfordI was talking about the string constant "ZZ" and not the Checkbox named xZZ. I tried to say that yoyr SQL request built with Delphi code is OK and DO NOT suffer of SQL injection risk. I was trying to say that if instead of the constant "ZZ" you want to use a value from a TEdit, then suddenly your code becomes at SQL injection risk and to avoid it you have to use a parametrized SQL request.

And as I hope I clarified, there was no string constant involved. Your warning of using a string value from a TEdit IS IN FACT a good thing to know, since it legitimizes the work in switching to Parameters, since in OTHER cases, I have done EXACTLY what you are warning me about. So, thanks for pointing out the risks my habits have exposed me to. GM

[Guest]

1 hour ago, Gary Mugford said:

Dany, the contained code won't work in BDE.

If you connect to a Paradox database, no, for sure not.

 

Can you issue SQL to a Paradox DB? I'd guess there's some unchecked local interpreter.

Edited May 1, 2021 by Guest

[Gary Mugford 12]

Dany,

 

  The SQL in BDE for Paradox is a sub-set of, IIRC, SQL-97. The important limitation is no sub-clauses. In fact, there's even a missing Parameters setting that I see in most Parameterization examples. It simply throws an error and leaving it out doesn't seem to affect things. (Params.ParamCheck := true; specifically).

 

  The template I'm using as I add the Params code to my code looks like this:

//Params.ParamCheck := true; // DOES NOT WORK IN D7!! Might in XE7??
Params.ParamByName('eDF').asDataType := ftDateTime;
Params.ParamByName('eDF').asDate  := eDateFrom.asDate; // value can also be used
Params.ParamByName('eDT').asDataType := ftDateTime;
Params.ParamByName('eDT').asDate  := eDateTo.asDate; // value can also be used

  I have a composite component that is built of a panel, a menu button and two date components from the indispensable ESB library. I call it DateRanger and the menu button in the middle provides quick resetting of the two dates to Last, Current and Next; Week, Month, Year and Quarter, as well as some occasionally asked for ranges like YearToDate, Last365Days and AllDates. Since I use this panel in a great many forms, I can afford a template to include, as examples, the use of the data from eDateFrom and eDateTo. There is nothing to do but change the the query variables to :qvZZ as theorized by François, the DataType ftString and then the value from the proposed TEdit. Then delete the next two lines. 

 

  At SOME point in time, I will end up working with NexusDB seriously. At that point, a fulll panoply of SQL language features will be made available. THEN, some of your code becomes available for me to experiment with. 

 

  IN REVIEW: Changing my code to use Parameters in the On The Fly creation is a good idea. Nobody has raised the specter of problems with query variable reuse, although I will be shying away from changing DataTypes whilst doing that. And I can continue to use On The Fly style.  The likelihood that anybody would want to disassemble my code as some start to cracking it for nefarious purposes is slim, maybe exceedingly slim, and hard to reach since the server it sits on is not accessible via the Internet. Strictly sneaker net. At least for most of it. 

 

  Thanks all for the help. GM

Edited May 1, 2021 by Gary Mugford

[Guest]

9 hours ago, Gary Mugford said:

The SQL in BDE for Paradox is a sub-set of, IIRC, SQL-97.

Yes, i remember now. So a local interpreter converts SQL.

Absolutely correct; with a full-blown RDBMS the "central" server interprets SQL. Always the same.

The various DACs have more or less "pre-processing" of the queries, also they usually parse them an instantiate (at prepare time) "helper cursors" for lookups and such.

You will love it!

In order to make you even more "excited", check out the concept called "CTE". Quite cool.

 

On a different note; with Paradox, if you are doing some kind of batch processing where more than one record and table is involved, you have to write quite complex code to "roll back" as other users might have changed something during the processing. Using an ACID compliant database you can scrap all of that client side and it will become so much cleaner done in PSQL. So this is why i would recommend to rewrite the beast instead of trying to migrate.

 

HTH

[Gary Mugford 12]

Dany,

 

  Last century, well back into last century, I devised an audit trail feature for my applications. All changes in the data were noted and documented in such a way that WHAT the changes were, were easily accessible and thus it mitigated what I called the "NotMe" problem (this plays off a regular feature in Bil Keane comic strips). The NotMe problem was the almost herd-like autonomic response to problems. The History Notes simply eliminated the issue since it was clear who, when and what. This was back in the early 80's and was a feature very valued by my customers. It didn't allow for rollback, but it did leave a change trail to be manually reversed. And it eliminated SOOOOO much angst in the office(s) over what happened. And that SINGLE feature probably kept me employed for as long as I have been, despite no academic background (well, ALMOST no school training) in computer programming and a stick in the mud adherence to not change with the times and keep current with language and database development. However, as I pass officially into senior citizen-hood, I've changed my mind and decided to try and be more au courant. 

 

  And with people such as yourself, who hold out helping hands, I MIGHT just stretch out my usefulness and put off the switch over to writing books JUST a little longer. Thanks again.

[Guest]

You should give ma e like or a bucket 🙂

 

More seriously; a real RDBMS has triggers. Me, being a FireBird-centric person, uses this https://www.upscene.com/fb_tracemanager/ to add logging to specific tables for specific clients. It's more of a "configuration" scenario than coding.

 

I have had my a** saved a lot of times with new employees @client's side that claim that they have lost information "entered" into my systems. With the logs i can say that "at that second you saved this, and at this time you changed that to this". And immediately all HR/CEO/Whateever-people stop bitching me. It's like magic! And the only info o have is by commits, i do not do client side telemetries at all (the reason being business-related).

 

I have also implemented a generic "changelog" grid using DevExpress, their grid has a functionality that allows you to switch the column TYPE between rows (yes). So i can list all changes WITH the lookup-editors and icons and all that. My client's bosses can see exactly who change a specific record from what to what and when.

 

...

 

If you are interested in ½ a year or 1 year hence i am going to release my "9:th generation" as an OpenSourced "Application Framework" project. It will probably never become very popular because of all the convolutedness and the dependency of a couple of non-free 3rd party components (like the DAC), the main idea is preparing my clients for my old age, actually. It's convenient to tell the CEO's that the code is out there if i happen to lie down under a bus or something.

 

When i worked with BDE i had OWL (long time back) so i went on for a couple of years without being able to adopt the more modern DACs that was built on the VCL. There were a lot of workarounds. Then i switched from C++ to OP and OWL to VCL. That was generation 3.

 

Good luck!

[Fr0sT.Brutal 900]

BTW, you might wish to get rid of some SQL constructing to have the query text solid with such kind of conditions:

Qr.SQL:='SELECT ... FROM SomeTable WHERE ( (:CheckFoo = 1) AND (SomeTable.Foo = :Foo) )';
Qr.ParamByName['CheckFoo'].Value := Ord(chbCheсkFoo.Checked);
Qr.ParamByName['Foo'].Value := IfThen(chbCheсkFoo.Checked, eFoo.Text, '');

 

 

[Gary Mugford 12]

Fr0st.Brutal,

 

I admit, most of the world does exactly as you suggest, although there might be a few who use the BeginUpdate and EndUpdate lines. For me, line-by-line SQL is more readable than the format you suggest. When I have errors in my SQL, and I DO have errors, it's better for the line number identification when I try the code directly into the SQL.lines of the BDE SQL query object. Since I am still a learner here in my dotage, directing me to the direct line of the error is of measurable value. Thank you for your suggestion, though. GM

[Guest]

I don't think i have any SQL in my .pas (a lot in dfm's though).

But...

 

SQL.Text :=

'Select ...' + #13!0 +

'From table t' + #13#10 +...

 

MMX and GExperts have functions, i think, for converting the SQL typed into an editor to pascal strings and vice-versa (?).

Use only when lacking a proper DAC.

[Fr0sT.Brutal 900]

8 minutes ago, Gary Mugford said:

Fr0st.Brutal,

 

I admit, most of the world does exactly as you suggest, although there might be a few who use the BeginUpdate and EndUpdate lines. For me, line-by-line SQL is more readable than the format you suggest. When I have errors in my SQL, and I DO have errors, it's better for the line number identification when I try the code directly into the SQL.lines of the BDE SQL query object. Since I am still a learner here in my dotage, directing me to the direct line of the error is of measurable value. Thank you for your suggestion, though. GM

Contrary, I changed all of my "SQL.Add(s)" to "SQL.Text := s". Even if I construct query conditionally, I collect it first as variable and then assign it to SQL.

[haentschman 92]

Quote

Contrary, I changed all of my "SQL.Add(s)" to "SQL.Text := s". Even if I construct query conditionally, I collect it first as variable and then assign it to SQL.

advertising

 

This is another way to manage SQL statements. The SQL are stored in separate files in a folder structure in the project. SQL can be tested in the preferred DBMS editor. 

Important: SQL Statements OUTSIDE of the Sourcecode (pas, dfm) in resources *.res. (without SQL.Add; SQL.Add; SQL.Add... ) 

 

 

 

Example:

...only 1 line for complete SQL.Text. 

function TDatabase.CreateDocument(Path: string): TDocument;
var
  Qry: TFDQuery;
  Document: TDocument;

  procedure FillDocument;
  begin
    // ! Achtung Reihenfolge wegen Properties die aus Vorhergehenden ermittelt werden
    Document := TDocument.Create;
    Document.State := sdsNormal;
    Document.ID := Qry.FieldByName('ID').AsInteger;

    Document.AddDate := Qry.FieldByName('AddDate').AsDateTime;
    Document.AddName := Qry.FieldByName('AddName').AsString;
    Document.AddYear := YearOf(Document.AddDate);
    Document.DocumentGroupString := Qry.FieldByName('DocumentGroupString').AsString;
    Document.DocumentTypeString := Qry.FieldByName('DocumentTypeString').AsString;
    Document.DocumentCaption := Qry.FieldByName('DocumentCaption').AsString;
    Document.DocumentChoice := TSEAMTools.GetDocumentChoiceType(Document.DocumentTypeString);
    Document.SendTypeFolder := Boolean(Qry.FieldByName('SendTypeFolder').AsInteger);
    Document.SendTypeUSB := Boolean(Qry.FieldByName('SendTypeUSB').AsInteger);
    Document.SendTypeMail := Boolean(Qry.FieldByName('SendTypeMail').AsInteger);
    Document.ReceiptDate := Qry.FieldByName('ReceiptDate').AsDateTime;
    Document.ServiceDate := Qry.FieldByName('ServiceDate').AsDateTime;
    Document.ReceiptNumber := Qry.FieldByName('ReceiptNumber').AsString;
    Document.ReceiptReceiver := Qry.FieldByName('ReceiptReceiver').AsString;
    Document.ReceiverReceiverName := Qry.FieldByName('ReceiverReceiverName').AsString;
    Document.Store := Qry.FieldByName('Store').AsString;
    Document.StoreName := Qry.FieldByName('StoreName').AsString;
    Document.StoreName_1 := Qry.FieldByName('StoreName_1').AsString;
    Document.StoreCountry := Qry.FieldByName('StoreCountry').AsString;
    Document.StorePostCode := Qry.FieldByName('StorePostCode').AsString;
    Document.StoreLocation := Qry.FieldByName('StoreLocation').AsString;
    Document.StoreStreet := Qry.FieldByName('StoreStreet').AsString;
    Document.ServicePartner := Qry.FieldByName('ServicePartner').AsString;
    Document.ModifiedDate := Qry.FieldByName('ModifiedDate').AsDateTime;
    Document.ModifiedName := Qry.FieldByName('ModifiedName').AsString;

    Document.OriginalFileName := Qry.FieldByName('OriginalFileName').AsString; // als Letztes wegen Setter
    Document.DocumentLocation := TTools.GetDocumentLocationType(Document.OriginalFileName);

    Result := Document;
  end;

begin
  Result := nil;

  Qry := CreateQuery;
  try
    // Pfad
    Qry.SQL.Text := GetSQLByName('SEAM_DOCUMENT_SELECT_PATH');
    Qry.ParamByName('FIN').AsString := Path;
    Qry.Open;

    if Qry.Eof then
    begin
      // Belegnummer
      Qry.SQL.Text := GetSQLByName('SEAM_DOCUMENT_SELECT_RECEIPT_NUMBER');
      Qry.ParamByName('REN').AsString := TToolsRegex.ExtractReceiptNumber(Path).Value;
      Qry.Open;

      if not Qry.Eof then
      begin
        FillDocument;
      end;
    end
    else
    begin
      FillDocument;
    end;
  finally
    Qry.Free;
  end;
end;

Edited May 5, 2021 by haentschman

[Gary Mugford 12]

On 5/4/2021 at 10:26 AM, Dany Marmur said:

I don't think i have any SQL in my .pas (a lot in dfm's though).

But...

 

SQL.Text :=

'Select ...' + #13!0 +

'From table t' + #13#10 +...

 

MMX and GExperts have functions, i think, for converting the SQL typed into an editor to pascal strings and vice-versa (?).

Use only when lacking a proper DAC.

Dany,

 

  Towards the end of MMX development before it went open source, I was an active 'suggester' of features. I might have been the first to suggest a tool breaking apart the With statement. I do use MMX for precisely the task you are talking about. Too bad active development in MMX no longer includes Delphi 7, but I'm such a small niche I can hardly expect to be catered to. 

 

  I've read the preferences here regarding SQL code keeping and I accept that my style is ... not that of the community. But the efforts of all of you are appreciated. I'll probably REMAIN a stick in the mud. Given my templates and leading /// notation atop query construction and execution, I find looking for SQL code in my .PAS files to be fairly easy. It's less fidgety than searching from DFMs. At least to me, that's the case. I'm a LITTLE intrigued by the DIMOMA SQL resource creator. It has the charm of having a default test SQL version that creates code usable by my report generator and then having a V2 that has query variables in it to use in Run Time work. I'm going to at least look at it. But the single DBMS is where I might run afoul of the licence since I want to convert Paradox to NexusDB and will need both in action for some time. Won't know whether what I do is actually within the licence parameters until I give it a test. 

 

  Thanks all. GM