Warning: HFS v2.x has a severe vulnerability

[LeoNeeson]

Hi everyone! This is a notice to all the users of HFS version 2.x (I will call it 'HFS2' for making it short). Recently, a severe vulnerability (CVE-2024-23692) was found in HFS2 (known to affect HFS v2.4.0 RC7 and HFS v2.3m). This information was kept private until now, to give it time to find a solution, but now I think it's time to make this notice public. This is only an informational message to let everyone know about this. Anyone with Pascal/Delphi knowledge could contribute to finding a fix.

We are discussing how to patch it, here:
https://github.com/drapid/hfs/issues/3

You could contribute by submitting code fixes to the source code, either on GitHub or here in the appropriate forum section: Programmers corner (opening a new thread there or leaving a comment here on this very same thread). If we find a correct fix (and since Rejetto will not update HFS2 anymore), perhaps we can build an unofficial "community" version for those who can't upgrade to HFS3.

Let's keep HFS v2.x alive, and...
...please do not panic.

Stay safe,
Leo.-

[LeoNeeson]

After spending several hours on last weekend, I'm happy to finally announce that I've come up with a simple (one line) solution to this macro vulnerability.

The following is a portion of 'main.pas' in 'hfs2.3m.src.zip'
Add the line marked in red, after line 5100 in 'main.pas'
(After line 5445 in v2.4 RC07, but is hasn't been tested)

  url:=conn.request.url; // The next line is a fix for CVE-2024-23692
  if anyMacroMarkerIn(url) then url:=encodeURL(xtpl(url,['%','#']));
  extractParams();
  url:=decodeURL(url);

This was my second 'impossible task' achieved or accomplished here (the first was helping to bring the 'logout' function to HFS), and now an attempt to fix this vulnerability. Those are the good things about programming: almost nothing is impossible with a lot of effort and dedication.

[rejetto]

that's great, congratulations with your achievement, Leo!
i cannot say anything about effectiveness of this fix, but i'm happy if you can find a solution.
Is the url the only way to use the vulnerability? Even if the POC of the CVE uses the url, be sure to consider the possibility of the attack coming from a header.
you prefer 2.3 over 2.4 ?

[LeoNeeson]

that's great, congratulations with your achievement, Leo!

Thanks, it means a lot that you like it, I appreciate it. To me, it's like an exercise to dip my toes on Delphi, although there is still a long way to go...

you prefer 2.3 over 2.4 ?

Not really, some parts of it only (2.4 has huge improvements), but mainly I'm using 2.3 in my tests because it is much easier for me to build it (using TD2006). And since it was the latest stable version published, it was the version I've used it as example.

Is the url the only way to use the vulnerability? Even if the POC of the CVE uses the url, be sure to consider the possibility of the attack coming from a header.

Yes, I know what you mean ('Host' header is not covered, for example), that's why I'm not completely happy with my 'single line' fix (although it works). That's why I'm testing a completely new fix (instead the previous code). See...


» Alternative method to stop this macro vulnerability:
Add the line marked in green, after line 5084 in 'main.pas' (v2.3m)

  runEventScript('pre-filter-request');

  // Check macro leaks, prevent hack attempts
  if anyMacroMarkerIn(conn.request.full) then
  begin
    data.disconnectReason:='Hack attempt blocked. This event has been logged!';
    add2log('Hack attempt blocked: '+ansiToUTF8(conn.request.url));
    getPage('deny', data);
    conn.reply.mode:=HRM_BAD_REQUEST;
    exit;
  end;

It works, but even then, it's just a simple check and stop, not a true 'urlvar' filtering (and I can't be 100% sure if it is enough or if some hacker could think a workaround to bypass this measure). And if the browser asks for the 'favicon.ico' along with the same request, it gets logged as hack attempt too (and I don't like this, and I have to think how to handle it, perhaps with 'urlCmd'). Alternatively, I was thinking of doing a 'stringReplace' of macro markers on 'request.full' at an earlier stage, right on the 'handleHeaderData' procedure (which also works, as second measure), but I don't like this approach, since it could mess with other parts of the code.

Well, I think that's all I will be working with this at the moment, I don't have too much time to go deeper analyzing this, it's only a start for now.