Skip to content

Commit

Permalink
Add denial of service protection inside CodeSystem iteration
Browse files Browse the repository at this point in the history
  • Loading branch information
Grahame Grieve committed Feb 12, 2025
1 parent 6be62d1 commit a50eff2
Show file tree
Hide file tree
Showing 16 changed files with 55 additions and 23 deletions.
1 change: 1 addition & 0 deletions exec/pack/Messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,7 @@ VALUESET_SUPPLEMENT_MISSING_one = Required supplement not found: {1}
VALUESET_SUPPLEMENT_MISSING_other = Required supplements not found: {1}
VALUESET_TOO_COSTLY = The value set ''{0}'' expansion has too many codes to display ({1})
VALUESET_TOO_COSTLY_COUNT = The value set ''{0}'' expansion has {2} codes, which is too many to display ({1})
CODESYSTEM_TOO_COSTLY_TIME = The operation ''{1}'' took too long to process (>{0}sec) (at {2})
VALUESET_TOO_COSTLY_TIME = The value set ''{0}'' {2} took too long to process (>{1}sec)
REQUEST_TOO_COSTLY_TIME = Request took too long to process (>{0}sec)
TX_SERVER_NOT_READY = The full data set is only {0}% loaded ({1} of {2}) for searching - repeat this query in a few minutes (max ~15min)');
Expand Down
2 changes: 1 addition & 1 deletion library/fhir/fhir_tx.pas
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ TTerminologyOperationContext = class (TTxOperationContext)
property reqId : String read FId;
property TimeTracker : TFslTimeTracker read FTimeTracker write SetTimeTracker;
function copy : TTerminologyOperationContext;
function deadCheck(var time : integer) : boolean;
function deadCheck(var time : integer) : boolean; override;
procedure seeContext(vurl : String);
procedure clearContexts;

Expand Down
40 changes: 25 additions & 15 deletions library/ftx/fhir_codesystem_service.pas
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface

uses
SysUtils, Classes, Generics.Defaults, Generics.Collections,
fsl_base, fsl_utilities, fsl_collections, fsl_http, fsl_lang, fsl_versions, fsl_fpc, fsl_logging, fsl_regex, fsl_i18n,
fsl_base, fsl_utilities, fsl_collections, fsl_http, fsl_lang, fsl_versions, fsl_fpc, fsl_logging, fsl_regex, fsl_i18n, fsl_threads,
fhir_objects, fhir_factory, fhir_common, fhir_cdshooks, fhir_utilities, fhir_features, fhir_uris,
ftx_service;

Expand Down Expand Up @@ -203,7 +203,7 @@ TFhirCodeSystemProvider = class (TCodeSystemProvider)
function doLocate(list : TFhirCodeSystemConceptListW; code : String; altOpt : TAlternateCodeOptions) : TFhirCodeSystemProviderContext; overload;
function getParent(ctxt : TFhirCodeSystemConceptW) : TFhirCodeSystemConceptW;
procedure FilterCodes(dest : TFhirCodeSystemProviderFilterContext; source : TFhirCodeSystemConceptListW; filter : TSearchFilterText);
procedure iterateCodes(base: TFhirCodeSystemConceptW; list: TFhirCodeSystemProviderFilterContext; filter : TCodeSystemCodeFilterProc; context : pointer; includeRoot : boolean; exception : TFhirCodeSystemConceptW = nil);
procedure iterateCodes(opContext : TTxOperationContext; op : String; base: TFhirCodeSystemConceptW; list: TFhirCodeSystemProviderFilterContext; filter : TCodeSystemCodeFilterProc; context : pointer; includeRoot : boolean; exception : TFhirCodeSystemConceptW = nil);
function locateParent(ctxt: TFHIRCodeSystemConceptW; code: String): String;
function locCode(list: TFhirCodeSystemConceptListW; code, synonym: String; altOpt : TAlternateCodeOptions): TFhirCodeSystemConceptW;
function getProperty(code : String) : TFhirCodeSystemPropertyW;
Expand All @@ -214,7 +214,7 @@ TFhirCodeSystemProvider = class (TCodeSystemProvider)
procedure iterateConceptsByRegex(src : TFhirCodeSystemConceptListW; regex: string; list: TFhirCodeSystemProviderFilterContext);
procedure iterateConceptsByEquality(positive : boolean; src : TFhirCodeSystemConceptListW; code: string; list: TFhirCodeSystemProviderFilterContext);

procedure listChildrenByProperty(code : String; list, children : TFhirCodeSystemConceptListW);
procedure listChildrenByProperty(opContext : TTxOperationContext; op : String; code : String; list, children : TFhirCodeSystemConceptListW);
protected
function sizeInBytesV(magic : integer) : cardinal; override;
public
Expand Down Expand Up @@ -1353,16 +1353,17 @@ function {TFhirCodeSystemProvider.}nonLeafCodes(context : pointer; concept: TFhi
result := concept.conceptList.Count > 0;
end;

procedure TFhirCodeSystemProvider.listChildrenByProperty(code: String; list, children: TFhirCodeSystemConceptListW);
procedure TFhirCodeSystemProvider.listChildrenByProperty(opContext : TTxOperationContext; op : String; code: String; list, children: TFhirCodeSystemConceptListW);
var
item : TFhirCodeSystemConceptW;
begin
for item in list do
begin
deadCheck(opContext, 'listChildrenByProperty', op);
if conceptHasProperty(item, 'http://hl7.org/fhir/concept-properties#parent', code) then
children.Add(item.link)
else if item.HasConcepts then
listChildrenByProperty(code, item.conceptList, children);
listChildrenByProperty(opContext, op, code, item.conceptList, children);
end;
end;

Expand Down Expand Up @@ -1446,7 +1447,7 @@ function TFhirCodeSystemProvider.version: String;
result := FCs.CodeSystem.version;
end;

procedure TFhirCodeSystemProvider.iterateCodes(base : TFhirCodeSystemConceptW; list : TFhirCodeSystemProviderFilterContext; filter : TCodeSystemCodeFilterProc; context : pointer; includeRoot : boolean; exception : TFhirCodeSystemConceptW = nil);
procedure TFhirCodeSystemProvider.iterateCodes(opContext : TTxOperationContext; op : String; base : TFhirCodeSystemConceptW; list : TFhirCodeSystemProviderFilterContext; filter : TCodeSystemCodeFilterProc; context : pointer; includeRoot : boolean; exception : TFhirCodeSystemConceptW = nil);
var
i : integer;
el : TFslList<TFHIRObject>;
Expand All @@ -1467,14 +1468,20 @@ procedure TFhirCodeSystemProvider.iterateCodes(base : TFhirCodeSystemConceptW; l

// 1. Add children in the heirarchy
for i := 0 to base.conceptList.count - 1 do
iterateCodes(base.conceptList[i], list, filter, context, true);
begin
deadCheck(opContext, 'iterate-codes-1', op);
iterateCodes(opContext, op, base.conceptList[i], list, filter, context, true);
end;

// 2. find any codes that identify this as a parent in their properties
cl := TFhirCodeSystemConceptListW.Create;
try
listChildrenByProperty(base.code, FCs.CodeSystem.conceptList, cl);
listChildrenByProperty(opContext, op, base.code, FCs.CodeSystem.conceptList, cl);
for i := 0 to cl.count - 1 do
iterateCodes(cl[i], list, filter, context, true);
begin
deadCheck(opContext, 'iterate-codes-2', op);
iterateCodes(opContext, op, cl[i], list, filter, context, true);
end;
finally
cl.free;
end;
Expand All @@ -1484,10 +1491,11 @@ procedure TFhirCodeSystemProvider.iterateCodes(base : TFhirCodeSystemConceptW; l
for e in el do
begin
ex := FFactory.wrapExtension(e.Link);
try
try
deadCheck(opContext, 'iterate-codes-3', op);
ctxt := doLocate(ex.value.primitiveValue, nil);
try
iterateCodes(TFhirCodeSystemProviderContext(ctxt).concept, list, filter, context, true);
iterateCodes(opContext, op, TFhirCodeSystemProviderContext(ctxt).concept, list, filter, context, true);
finally
ctxt.free;
end;
Expand Down Expand Up @@ -1677,6 +1685,7 @@ function TFhirCodeSystemProvider.filter(opContext : TTxOperationContext; forExpa
cc : TFhirCodeSystemConceptW;
includeRoot : boolean;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if (op in [foIsA, foDescendentOf]) and ((prop = 'concept') or (prop = 'code')) then
begin
code := doLocate(value, nil);
Expand All @@ -1690,7 +1699,7 @@ function TFhirCodeSystemProvider.filter(opContext : TTxOperationContext; forExpa
includeRoot := false;
result := TFhirCodeSystemProviderFilterContext.Create;
try
iterateCodes(code.concept, result as TFhirCodeSystemProviderFilterContext, allCodes, nil, includeRoot);
iterateCodes(opContext, 'filter('+prop+CODES_TFhirFilterOperator[op]+value+')', code.concept, result as TFhirCodeSystemProviderFilterContext, allCodes, nil, includeRoot);
result.link;
finally
result.free;
Expand All @@ -1711,7 +1720,7 @@ function TFhirCodeSystemProvider.filter(opContext : TTxOperationContext; forExpa
result := TFhirCodeSystemProviderFilterContext.Create;
try
for cc in FCs.CodeSystem.conceptList do
iterateCodes(cc, result as TFhirCodeSystemProviderFilterContext, allCodes, code.concept, true);
iterateCodes(opContext, 'filter('+prop+CODES_TFhirFilterOperator[op]+value+')', cc, result as TFhirCodeSystemProviderFilterContext, allCodes, code.concept, true);
result.link;
finally
result.free;
Expand All @@ -1732,6 +1741,7 @@ function TFhirCodeSystemProvider.filter(opContext : TTxOperationContext; forExpa
begin
code := doLocate(value, nil);
try
deadCheck(opContext, 'filter-1', 'filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if code = nil then
raise ETerminologyError.Create('Unable to locate code '+value, itUnknown)
else
Expand All @@ -1754,9 +1764,9 @@ function TFhirCodeSystemProvider.filter(opContext : TTxOperationContext; forExpa
try
for cc in FCs.CodeSystem.conceptList do
if value = 'true' then
iterateCodes(cc, result as TFhirCodeSystemProviderFilterContext, nonLeafCodes, nil, true)
iterateCodes(opContext, 'filter('+prop+CODES_TFhirFilterOperator[op]+value+')', cc, result as TFhirCodeSystemProviderFilterContext, nonLeafCodes, nil, true)
else
iterateCodes(cc, result as TFhirCodeSystemProviderFilterContext, leafCodes, nil, true);
iterateCodes(opContext, 'filter('+prop+CODES_TFhirFilterOperator[op]+value+')', cc, result as TFhirCodeSystemProviderFilterContext, leafCodes, nil, true);
result.link;
finally
result.free;
Expand Down
3 changes: 2 additions & 1 deletion library/ftx/ftx_lang.pas
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface

uses
SysUtils, Classes, Generics.Collections,
fsl_utilities, fsl_stream, fsl_base, fsl_http, fsl_lang,
fsl_utilities, fsl_stream, fsl_base, fsl_http, fsl_lang, fsl_threads,
fhir_objects, fhir_common, fhir_features, fhir_uris,
ftx_service;

Expand Down Expand Up @@ -299,6 +299,7 @@ function TIETFLanguageCodeServices.filter(opContext : TTxOperationContext; forEx
var
i : integer;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
i := StringArrayIndexOfSensitive(CODES_TIETFLanguageComponent, prop);
if (i >= 0) and (op = foExists) and ((value = 'true') or (value = 'false')) then
result := TIETFLanguageCodeFilter.Create(TIETFLanguageComponent(i), value = 'true')
Expand Down
1 change: 1 addition & 0 deletions library/ftx/ftx_loinc_services.pas
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,7 @@ function TLOINCServices.filter(opContext : TTxOperationContext; forExpansion, fo
s : string;
d : String;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
d := prop+' '+CODES_TFhirFilterOperator[op]+' '+value;
c := FDB.getConnection('filterBySQL');
try
Expand Down
1 change: 1 addition & 0 deletions library/ftx/ftx_sct_services.pas
Original file line number Diff line number Diff line change
Expand Up @@ -5262,6 +5262,7 @@ function TSnomedProvider.filter(opContext : TTxOperationContext; forExpansion, f
var
id : UInt64;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
result := nil;
if (prop = 'concept') and FSct.StringIsId(value, id) then
if op = foIsA then
Expand Down
10 changes: 10 additions & 0 deletions library/ftx/ftx_service.pas
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ ETerminologyError = class (EFslException) // problem in terminology operation
TTxOperationContext = class abstract (TFslObject)
public
procedure log(note : String); virtual; abstract;
function deadCheck(var time : integer) : boolean; virtual; abstract;
end;

TCodeSystemProviderContext = class (TFslObject)
Expand Down Expand Up @@ -240,6 +241,7 @@ TCodeSystemProvider = class abstract (TFslObject)
FLanguages : TIETFLanguageDefinitions;
FI18n : TI18nSupport;
procedure setDefLang(value : TIETFLang);
procedure deadCheck(opContext: TTxOperationContext; place, op: String);
public
constructor Create(languages : TIETFLanguageDefinitions; i18n : TI18nSupport);
destructor Destroy; override;
Expand Down Expand Up @@ -902,6 +904,14 @@ procedure TCodeSystemProvider.setDefLang(value: TIETFLang);
FDefLang := value;
end;

procedure TCodeSystemProvider.deadCheck(opContext: TTxOperationContext; place, op: String);
var
time : integer;
begin
if opContext.deadCheck(time) then
raise ETooCostly.create(FI18n.translate('CODESYSTEM_TOO_COSTLY_TIME', nil, [inttostr(time), op, place]));
end;

constructor TCodeSystemProvider.Create(languages: TIETFLanguageDefinitions; i18n : TI18nSupport);
begin
inherited Create;
Expand Down
3 changes: 2 additions & 1 deletion library/ftx/ftx_ucum_services.pas
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

Uses
SysUtils, Classes,
fsl_base, fsl_utilities, fsl_collections, fsl_stream, fsl_xml, fsl_ucum, fsl_http, fsl_lang, fsl_i18n,
fsl_base, fsl_utilities, fsl_collections, fsl_stream, fsl_xml, fsl_ucum, fsl_http, fsl_lang, fsl_i18n, fsl_threads,
ftx_ucum_handlers, ftx_ucum_validators, ftx_ucum_expressions, ftx_ucum_base,
fhir_common, fhir_features, fhir_uris,
fhir_cdshooks,
Expand Down Expand Up @@ -1053,6 +1053,7 @@ function TUcumServices.TotalCount: integer;

function TUcumServices.filter(opContext : TTxOperationContext; forExpansion, forIteration : boolean; prop: String; op: TFhirFilterOperator; value: String; prep : TCodeSystemProviderFilterPreparationContext): TCodeSystemProviderFilterContext;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if (prop = 'canonical') and (op in [foEqual]) then
result := TUcumFilterContext.create(value)
else
Expand Down
4 changes: 2 additions & 2 deletions release-notes.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Change Notes:

*
* Add denial of service protection inside CodeSystem iteration

## Conformance Notes:

*
* tx.fhir.org passed 300 HL7 terminology service tests (mode 'tx.fhir.org', tests v1.7.5, runner v6.5.8)
3 changes: 2 additions & 1 deletion server/tx/tx_areacode.pas
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface

uses
SysUtils, Classes,
fsl_utilities, fsl_http, fsl_lang, fsl_base, fsl_stream, fsl_i18n,
fsl_utilities, fsl_http, fsl_lang, fsl_base, fsl_stream, fsl_i18n, fsl_threads,
fhir_objects, fhir_common, fhir_features,
ftx_service;

Expand Down Expand Up @@ -544,6 +544,7 @@ function TAreaCodeServices.filter(opContext : TTxOperationContext; forExpansion,
res : TAreaCodeConceptFilter;
c : TAreaCodeConcept;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if ((prop = 'type') or (prop = 'class')) and (op = foEqual) then
begin
res := TAreaCodeConceptFilter.Create;
Expand Down
3 changes: 2 additions & 1 deletion server/tx/tx_countrycode.pas
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface

uses
SysUtils, Classes,
fsl_utilities, fsl_base, fsl_stream, fsl_http, fsl_fpc, fsl_lang, fsl_regex, fsl_i18n,
fsl_utilities, fsl_base, fsl_stream, fsl_http, fsl_fpc, fsl_lang, fsl_regex, fsl_i18n, fsl_threads,
fhir_objects, fhir_common, fhir_features, fhir_uris,
ftx_service;

Expand Down Expand Up @@ -1058,6 +1058,7 @@ function TCountryCodeServices.filter(opContext : TTxOperationContext; forExpansi
list : TCountryCodeConceptFilter;
concept : TCountryCodeConcept;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if (op = foRegex) and (prop = 'code') then
begin
list := TCountryCodeConceptFilter.Create;
Expand Down
1 change: 1 addition & 0 deletions server/tx/tx_cpt.pas
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ function TCPTServices.filter(opContext : TTxOperationContext; forExpansion, forI
item : TCPTConcept;
b : boolean;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
// filters supported
// * modified = false
// * modifier = true / false
Expand Down
3 changes: 2 additions & 1 deletion server/tx/tx_iso_4217.pas
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ interface

uses
SysUtils, Classes,
fsl_utilities, fsl_base, fsl_stream, fsl_lang, fsl_http, fsl_i18n,
fsl_utilities, fsl_base, fsl_stream, fsl_lang, fsl_http, fsl_i18n, fsl_threads,
fhir_objects, fhir_common, fhir_features,
ftx_service;

Expand Down Expand Up @@ -250,6 +250,7 @@ function TIso4217Services.filter(opContext : TTxOperationContext; forExpansion,
res : TIso4217ConceptFilter;
c : TIso4217Currency;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if (prop = 'decimals') and (op = foEqual) then
begin
res := TIso4217ConceptFilter.Create;
Expand Down
1 change: 1 addition & 0 deletions server/tx/tx_ndc.pas
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,7 @@ function TNDCServices.filter(opContext : TTxOperationContext; forExpansion, forI
ctxt : TNDCFilterPreparationContext;
res : TNDCFilterContext;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
res := nil;
try
ctxt := prep as TNDCFilterPreparationContext;
Expand Down
1 change: 1 addition & 0 deletions server/tx/tx_omop.pas
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ function TOMOPServices.filter(opContext : TTxOperationContext; forExpansion, for
var
f : TOMOPFilter;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
if (prop = 'domain') and (op = foEqual) then
begin
f := TOMOPFilter.Create;
Expand Down
1 change: 1 addition & 0 deletions server/tx/tx_rxnorm.pas
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,7 @@ function TUMLSServices.filter(opContext : TTxOperationContext; forExpansion, for
res : TUMLSFilter;
ok : boolean;
begin
SetThreadStatus(ClassName+'.filter('+prop+CODES_TFhirFilterOperator[op]+value+')');
prop := prop.toUpper;
res := TUMLSFilter.Create;
try
Expand Down

0 comments on commit a50eff2

Please sign in to comment.