Improve handling of cli_chkpua

[shawniverson]

Cleaned up code mentioned as follows from clamav-users list.

Modified code courtesy of "G.W. Haywood" clamav@jubileegroup.co.uk

Hi there,

On Sat, 19 Nov 2022, Andy Schmidt via clamav-users wrote:

Unfortunately, while will specifying "Win.Packer" or even "PUA.Win.Packer" will APPEAR to work, the program logic in ExcludePUA is completely faulty (almost arbitrary).

Yes, it WILL exclude those two - but the problem is, it will exclude GENERICALLY EVERYTHING ELSE (e.g., ALL "Win" or ALL "PUA") - in which case you might as well turn off the entire PUA feature!

I finally remembered that I had been down this exact rabbit hole years ago - and found this bug report:
https://bugzilla.clamav.net/show_bug.cgi?id=12632#c5

It seems the entire PUA feature is a step-child - by now, not even the config sample and documentation are current. Maybe its time to pull the plug on it, if no one is taking ownership to making it work?

(Yes, I realize the answer is to just "contribute" the fixes myself - but that assumes that every ClamAV user is also a C++ programmer, which I am not.)

The problem in the currently released code is that a 'category' turns
out to be only the second piece of a string made up of potentially
several dot-separated pieces. It needs more granularity.

Try replacing the function cli_chkpua() in .../libclamav/readdb.c with this:

static int cli_chkpua(const char *signame, const char *pua_cats, unsigned int options)
{
     // 2022.11.20 == GWH ==  "Categories" are dot-separated strings.
     // The string in the 'pua_cats' argument contains the PUA "categories" which are to be (depending on the configuration) included or excluded.
     // The category name in 'cat' is to be the string between the first and last dots in the signature string held in the 'signame' argument.
     // We will extract the category thus defined from the string in 'signame' and then look for this category within in the string in pua_cats.
     char cat[32], *cat_pt, *pt1, *pt2, *endsig;
     const char *sig;
     int ret;

     cli_dbgmsg("cli_chkpua: Checking signature [%s]\n", signame);

     if (strncmp(signame, "PUA.", 4)) {
         cli_dbgmsg("Skipping signature %s - no PUA prefix\n", signame);
         return 1;
     }
     sig = signame + 3;
     if (!(pt1 = strchr(sig + 1, '.'))) {                                        // pt1 points to the FIRST dot in the string in 'signame' if there is one, else NULL.
         cli_dbgmsg("Skipping signature %s - bad syntax\n", signame);
         return 1;
     }
     if ( (pt2 = strrchr(sig + 1, '.')) != pt1 ) {                               // pt2 points to the LAST dot in the string in 'signame' if there is one, else NULL.
         cli_dbgmsg("Signature has at least three dots [%s]\n", signame);        // If they happen to be the same dot, there are only two of them in the signature.
     }
//  else {
//      cli_dbgmsg("Seems signature only has two dots [%s]\n", signame);
//  }
     if ((unsigned int)(pt1 - sig + 2) > sizeof(cat)) {
         cli_dbgmsg("Skipping signature %s - too long category name, length approaching %d characters\n", signame, (unsigned int)(pt1 - sig + 2) );
         return 1;
     }
//  else {
//      cli_dbgmsg("Allowing signature %s; OK length category name, length approaching %d characters\n", signame, (unsigned int)(pt1 - sig + 2) );
//  }
     if ((unsigned int)(pt2 - sig + 2) > sizeof(cat)) {
         cli_dbgmsg("Skipping signature %s - too long category name, length approaching %d characters\n", signame, (unsigned int)(pt2 - sig + 2));
         return 1;
     }
//  else {
//      cli_dbgmsg("Allowing signature %s; OK length category name, length approaching %d characters\n", signame, (unsigned int)(pt2 - sig + 2));
//  }

     endsig = strrchr(sig, '.');
     strncpy(cat, sig, strlen(sig) - strlen(endsig) + 1);                        // Put in 'cat' the string between the first and last dots in sig, including the dots.
     cat[strlen(sig) - strlen(endsig) + 1] = 0;
     cat_pt                = strstr(pua_cats, cat);                              // Find if cat exists in pua_cats.
//  cli_dbgmsg("cli_chkpua:           pua_cats=[%s]\n", pua_cats                 );
//  cli_dbgmsg("cli_chkpua:            signame=[%s]\n", signame                  );
     cli_dbgmsg("cli_chkpua:                cat=[%s]\n", cat                      );
     cli_dbgmsg("cli_chkpua:                sig=[%s]\n", sig                      );
//  cli_dbgmsg("cli_chkpua:             endsig=[%s]\n", endsig                   );
//  cli_dbgmsg("cli_chkpua:             cat_pt=[%s]\n", cat_pt  ? cat_pt : "null");
//  cli_dbgmsg("cli_chkpua:                pt1=[%s]\n", pt1     ? pt1 : "null"   );
//  cli_dbgmsg("cli_chkpua:                pt2=[%s]\n", pt2     ? pt2 : "null"   );
     if (options & CL_DB_PUA_INCLUDE)
         ret = cat_pt ? 0 : 1;
     else
         ret = cat_pt ? 1 : 0;

     if (ret)
       cli_dbgmsg("Skipping PUA signature %s - excluded category %s\n", signame, cat);
     return ret;
}

No promises, but it's loaded the DB OK here. Please feel free to
correct mistakes in this and push to Github or whatever.

HTH

--

73,
Ged.

[ragusaa]

I have played with this PR a little, and I am finding that if I use just the category, then a signature is not excluded. For example.

If I have signatures PUA.Cat.subcat.sig, and my command is --exclude-pua=Cat, they are not excluded. It looks like the arguments for strstr are backwards. pua_cats is actually the 'needle', and cat is the 'haystack' (It is currently incorrect in main).

I have tried switching this call to 'cat_pt = strstr(cat, pua_cats);', and it excludes as expected.

[shawniverson]

I will flip these two params and update the PR

[shawniverson]

PR updated

[ragusaa]

Sorry for the delay in re-review. Approving.