View Issue Details

IDProjectCategoryView StatusLast Update
0034538FPCFCLpublic2018-11-29 16:22
Reporterchmod222Assigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformAnyOSAnyOS VersionAny
Product Version3.3.1Product Build 
Target Version3.2.0Fixed in Version3.3.1 
Summary0034538: fcl-web: httproute.HTTPRouter.StringToRouteMethod segfaults on unknown HTTP method, which can be exploited remotely
DescriptionAn fcl-web application can be instantly segfaulted remotely by sending it an HTTP request with a non-supported request verb, such as "PATCH", an uncommonly used but still valid HTTP verb.

This is due to an underflow on httproute.pas:425 caused by an off by one in the loop condition, where the Pred() call in the previous iteration lowers the enum value below the actual enum minimum, causing a crash in the next iteration.

I have attached a patch for this issue which I have tested on the current trunk build.
Steps To ReproduceCall HTTPRouter.StringToRouteMethod with any unsupported string
TagsNo tags attached.
Fixed in Revision40393
FPCOldBugId
FPCTarget
Attached Files
  • 0001-fix-httproute-segfault.diff (536 bytes)
    Index: packages/fcl-web/src/base/httproute.pp
    ===================================================================
    --- packages/fcl-web/src/base/httproute.pp	(Revision 40283)
    +++ packages/fcl-web/src/base/httproute.pp	(Arbeitskopie)
    @@ -422,7 +422,7 @@
     begin
       Result:=High(TRouteMethod);
       MN:=Uppercase(S);
    -  While (Result>=Low(TRouteMethod)) and (RouteMethodNames[Result]<>MN) do
    +  While (Result>Low(TRouteMethod)) and (RouteMethodNames[Result]<>MN) do
         Result:=Pred(Result);
       if Result=rmAll then Result:=rmUnknown;
     end;
    
  • testcase.pas (117 bytes)
    program httpproject1;
    
    {$mode objfpc}{$H+}
    
    uses
      httproute;
    
    begin
     HTTPRouter.StringToRouteMethod('PATCH');
    end.
    
    
    testcase.pas (117 bytes)

Activities

chmod222

2018-11-11 22:51

reporter  

0001-fix-httproute-segfault.diff (536 bytes)
Index: packages/fcl-web/src/base/httproute.pp
===================================================================
--- packages/fcl-web/src/base/httproute.pp	(Revision 40283)
+++ packages/fcl-web/src/base/httproute.pp	(Arbeitskopie)
@@ -422,7 +422,7 @@
 begin
   Result:=High(TRouteMethod);
   MN:=Uppercase(S);
-  While (Result>=Low(TRouteMethod)) and (RouteMethodNames[Result]<>MN) do
+  While (Result>Low(TRouteMethod)) and (RouteMethodNames[Result]<>MN) do
     Result:=Pred(Result);
   if Result=rmAll then Result:=rmUnknown;
 end;

chmod222

2018-11-11 22:58

reporter  

testcase.pas (117 bytes)
program httpproject1;

{$mode objfpc}{$H+}

uses
  httproute;

begin
 HTTPRouter.StringToRouteMethod('PATCH');
end.

testcase.pas (117 bytes)

Thaddy de Koning

2018-11-12 12:02

reporter   ~0111929

Last edited: 2018-11-12 12:42

View 4 revisions

Confirmed.
But I would also "patch" TRouteMethod since rmPatch is the last and is the only one missing.
  TRouteMethod = (rmUnknown,rmAll,rmGet,rmPost,rmPut,rmDelete,rmOptions,rmHead, rmTrace, rmPatch);

and:
Const
  RouteMethodNames : Array[TRouteMethod] of String = ('','','GET','POST','PUT','DELETE','OPTIONS','HEAD','TRACE','PATCH');

The class function then becomes even safer, like this:
class function THTTPRouter.StringToRouteMethod(const S: String): TRouteMethod;
Var
  MN : String;
  Result, i:TRouteMethod;
begin
  MN := UpperCase(S);
  Result:=rmUnknown;
  for i in TRouteMethod do
    if RouteMethodNames[i] = MN then
    begin
      Result := i;
      exit;
    end;
end;

Michael Van Canneyt

2018-11-29 16:22

administrator   ~0112260

Checked and applied the patch, thank you very much !

Issue History

Date Modified Username Field Change
2018-11-11 22:51 chmod222 New Issue
2018-11-11 22:51 chmod222 File Added: 0001-fix-httproute-segfault.diff
2018-11-11 22:58 chmod222 File Added: testcase.pas
2018-11-12 12:02 Thaddy de Koning Note Added: 0111929
2018-11-12 12:08 Thaddy de Koning Note Edited: 0111929 View Revisions
2018-11-12 12:39 Thaddy de Koning Note Edited: 0111929 View Revisions
2018-11-12 12:42 Thaddy de Koning Note Edited: 0111929 View Revisions
2018-11-12 13:25 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-11-12 13:25 Michael Van Canneyt Status new => assigned
2018-11-29 16:22 Michael Van Canneyt Fixed in Revision => 40393
2018-11-29 16:22 Michael Van Canneyt Note Added: 0112260
2018-11-29 16:22 Michael Van Canneyt Status assigned => resolved
2018-11-29 16:22 Michael Van Canneyt Fixed in Version => 3.3.1
2018-11-29 16:22 Michael Van Canneyt Resolution open => fixed
2018-11-29 16:22 Michael Van Canneyt Target Version => 3.2.0