View Issue Details

IDProjectCategoryView StatusLast Update
0034538FPCFCLpublic2018-11-29 15:22
Reporterchmod222 Assigned ToMichael Van Canneyt  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformAnyOSAny 
Product Version3.3.1 
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

Activities

chmod222

2018-11-11 21: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 21: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 11:02

reporter   ~0111929

Last edited: 2018-11-12 11: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 15:22

administrator   ~0112260

Checked and applied the patch, thank you very much !

Issue History

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