Skip to main content

Exim - Taint mismatch, string_nextinlist

Comments

16 comments

  • cPRex Jurassic Moderator
    Hey hey! We've got case CPANEL-36175 open for this issue, but the root of the problem is actually the way that Exim handles string expansions. That code you mention in the additional ACLs area is likely related to the issue. Here's some thoughts directly from Exim on these changes:
    0
  • sparek-3
    As long as you're aware of it - I guess that rules out that it's some of my exim customizations I have in place (at least until CPANEL-36175 is resolved and I'm still having issues). The string expansions and tainted data seemed to throw a lot of people between Exim 4.93 and Exim 4.94 - that's why I wonder if cPanel vetted all of their Exim Configuration ACL configurations when the update from Exim 4.93 and Exim 4.94 happened - it would seem that at least some of these were missed.
    0
  • cPRex Jurassic Moderator
    As far as I know, all the defaults were checked as part of the change, but it's always possible there are undiscovered issues.
    0
  • sparek-3
    I do see where there's some talk of similar issues on the Exim list from around June 2020 - apparently they pushed out a fix around that time. Whether or not the cPanel Exim is using the Exim with that fix, I can't tell. Although it looks like Exim was updated to Exim 4.94 in December 2020. There's also:
    0
  • cPRex Jurassic Moderator
    I've reached out to our Email team to see if I can get more details on this so I'll reply once I have that!
    0
  • sparek-3
    It seems cPanel already had a fix, just not deployed fully. It looks like the line: [font="courier new">[plain]set acl_m1 = ${if eq{$domain}{$primary_hostname}{${sg{$local_part}{\N[/+].*\N}{}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}[/plain] Just needs to be changed to: [font="courier new">[plain]set acl_m1 = ${if eq{$domain}{$primary_hostname}{${perl{untaint}{${sg{$local_part}{\N[/+].*\N}{}}}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}[/plain] When trying to think of possible resolutions to this - I thought about making a perl subroutine that just returns the argument that it was passed, hopefully this would satisfy the taintedness of the variable. Then when I was looking at this, I found that cPanel already had an untaint subroutine written (or at least one exists on my servers in /etc/exim.pl.local). So then just passing this variable through the untaint() subroutine - seems to fix the issue for me. This raises a couple of questions for me. 1) Is this safe to use? Why isn't cPanel configuring Exim to use this line? Or is that just an oversight on cPanel's part? 2) Where else is tainted $local_part being used in Exim that might create a simliar issue?
    0
  • sparek-3
    I suppose if you REALLY want to be proper about it - you should actually do a check in the perl subroutine and insure that the user being passed is actually a user. Create a new perl subroutine - [font="courier new">[plain]/usr/local/cpanel/etc/exim/perl/untaint_user_lookup[/plain] - with: [font="courier new">[plain] ## Do a getpwnam lookup to insure that the user is a user sub untaint_user_lookup { my($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell) = getpwnam($_[0]); return ($name ? $name : "nobody"); }[/plain] Then rebuild the Exim configuration [font="courier new">[plain]/scripts/buildeximconf[/plain] Then edit the [font="courier new">[plain]/etc/exim.conf[/plain] (probably a more proper way to do this ... you have to edit it after /scripts/buildeximconf because direct changes to /etc/exim.conf have been lost) ... and change [font="courier new">[plain]set acl_m1 = ${if eq{$domain}{$primary_hostname}{${sg{$local_part}{\N[/+].*\N}{}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}[/plain] to [font="courier new">[plain]set acl_m1 = ${if eq{$domain}{$primary_hostname}{${perl{untaint_user_lookup}{${sg{$local_part}{\N[/+].*\N}{}}}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}[/plain] This will insure that a correct username gets returned... although, I can't think of a scenario where a correct username would not get returned. Because this variable only gets set if /home/$localpart/.spamassassinenable - exists. But if... for whatever reason... /home/$localpart/.spamassassinenable exists but $localpart is NOT a valid user, then this subroutine will return nobody and will run SpamAssassin as the user nobody for this check. I suppose this could also get hit if /etc/global_spamassassin_enable exists on the server. The condition statement: [font="courier new">[plain]condition = ${if exists{/etc/global_spamassassin_enable}{1}{${if exists{${extract{5}{::}{${lookup passwd{${if eq{$domain}{$primary_hostname}{${sg{$local_part}{\N[/+].*\N}{}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}}}}}/.spamassassinenable}}}}[/plain] isn't exactly clear for me. Looks to me like there should be an OR in there some where, or maybe it defaults to an OR if one isn't specified. I guess this line is saying "If /etc/global_spamassassin_enable exists OR (lookup the homedir of $local_part if $domain is $primary_hostname else lookup the homedir of the user attached to $domain from /etc/userdomains, if that homedir/.spamassassinenable exists) - then this condition is true. (Exim if statements are a PAIN!) Not sure if this subroutine really gains anything. It just feels empty to pass an argument to a perl subroutine and have it return that argument without any checks. This way the untaint_user_lookup() subroutine is actually insuring that the argument passed is a real user on the server. The [font="courier new">[plain]set acl_m1 = ${if eq{$domain}{$primary_hostname}{${sg{$local_part}{\N[/+].*\N}{}}}{${lookup{$domain}lsearch{/etc/userdomains}}}}[/plain] part is failing for [plain]linuxusername@serverhostname[/plain] because no lookup is being done on the $local_part to insure that it's a clean variable. The [font="courier new">[plain]${lookup{$domain}lsearch{/etc/userdomains}}[/plain] passes because you're returning a value based on a lookup. This variable cleanliness requirement was introduced in Exim 4.94.
    0
  • sparek-3
    Last one, I think - although I can't promise You can make this change persistent by replacing default_spam_scan_check with a custom_begin_spam_scan_check with the updated set acl_m1 statement. All steps: 1) Create the untaint_user_lookup perl subroutine that we'll use to untaint the $local_part: [font="courier new">[plain]cat >/usr/local/cpanel/etc/exim/perl/untaint_user_lookup <custom_begin_spam_scan_check that essentially copies everything from the default_spam_scan_check block, except with the updated acl_m1. Note, if you happen to already have stuff in a custom_begin_spam_scan_check, then doing this step will overwrite that code. [font="courier new">[plain]cat >/usr/local/cpanel/etc/exim/acls/ACL_SPAM_SCAN_CHECK_BLOCK/custom_begin_spam_scan_check <default_spam_scan_check block: [font="courier new">[plain]sed -i "s/default_spam_scan_check=1/default_spam_scan_check=0/g" /etc/exim.conf.localopts[/plain] 4) And enable this new custom_begin_spam_scan_check block: [font="courier new">[plain]sed -i "s/custom_begin_spam_scan_check=0/custom_begin_spam_scan_check=1/g" /etc/exim.conf.localopts[/plain] 5) Rebuild the Exim Configuration: [font="courier new">[plain]/scripts/buildeximconf[/plain] 6) And restart Exim: [font="courier new">[plain]/scripts/restartsrv_exim[/plain]
    0
  • cPRex Jurassic Moderator
    I heard back from the team and the devs are working on a similar case, CPANEL-36663, which will also fix this issue as part of their work. I don't have any type of timeline, but it's definitely on their radar. The support page to follow along for this case is listed here: cPanel
    0
  • cPRex Jurassic Moderator
    Update - this is scheduled to be fixed in a future release of version 96. I don't have a specific version number available just yet, but I'll post once I hear something!
    0
  • cPRex Jurassic Moderator
    Update - the fix is scheduled to be included in 96.0.5.
    0
  • sparek-3
    So, again, no fix for cPanel 94? Then what's the point of having an TLS release? I just went ahead and applied my own fix to this on all of our servers. It was quicker than waiting for a cPanel resolution and now seemingly the only resolution unless I want to upgrade to cPanel 96. This is also why I'm reluctant to come forward with issues I face. It's easier for me to just write my own workaround, rather than explain the issues to cPanel and wait for them to push out a fix and then even longer for that fix to reach the branch of cPanel I'm using. Like I said in the other thread - cPanel really needs to look at their version/branching system.
    0
  • cPRex Jurassic Moderator
    @sparek-3 - you're a main contributor and I always look forward to hearing from you. Your replies are detailed, well thought out, and usually point me right to the issue I need to check. It's very valuable to get feedback like this. Just because it hasn't moved to LTS yet doesn't mean it never will, and even simple fixes go through much more testing than I think most realize, so they often aren't quick. I've asked the team if this is something that will get moved to 94, and I'll update once I hear back.
    0
  • sparek-3
    I guess that's fair. I guess I just wonder why nobody already in cPanel 96 never noticed the issue. I still think there are issues with getting pre-STABLE/LTS releases properly vetted before those releases get sent to STABLE/LTS. But maybe there's more going on behind the scenes than I'm not aware of.
    0
  • cPRex Jurassic Moderator
    Update - at this time the fix is scheduled to be included in 94.0.9 when that is released.
    0
  • cPRex Jurassic Moderator
    This got bumped to 94.0.11.
    0

Please sign in to leave a comment.