Re: [exim] Does exim4's `${sqlite_quote ... }` expansion de-…

Góra strony
Delete this message
Reply to this message
Autor: Nick
Data:  
Dla: Jeremy Harris, Nick via Exim-users
Temat: Re: [exim] Does exim4's `${sqlite_quote ... }` expansion de-taint the expanded value?
Thank you, Andrew and Jeremy - I was stuck on this, and your help took
me a step or two closer.

I now have my exim-disposable-aliases filter working, although I had to
chop a couple of other things out. It's also packaged as a .deb.


Some comments -

Regarding Jeremy's advice to "See the preferred syntax [in the
documentation, 26. More about SQLite]". I had already read this, but I
confess it was unclear to me what it was trying to say.

Specifically this paragraph:

> There are two ways of specifying the file. The first is is by using
> the sqlite_dbfile main option. The second, which allows separate files
> for each query, is to use an option appended, comma-separated, to the
> “sqlite” lookup type word. The option is the word “file”, then an
> equals, then the filename. The filename in this case cannot contain
> whitespace or open-brace charachters.


Not knowing exactly what "option" meant, I didn't totally follow what
the "second way" was. I'm happy to try and submit an amendment to the
documentation, if it helps, and someone can point me at the right way to
do that. I did look for submission advice in the Exim docs/website, and
I didn't find any.

Andrew supplied me with an example of the second syntax, so I was than
able to do this more confidently. Thanks Andrew.


Next, in adjusting the syntax, I discovered that Exim 4.94.2, as
packaged currently in Debian 11, does not seem to support the `file=`
option syntax. I got a different error:

    111334 disposable_aliases router: defer for
test.wu-lee@???
    111334   message: error in filter file: failed to expand "${lookup
sqlite,file=/var/spool/exim4/db/disposable-aliases.db { select
default_remaining from stem_configs where stem =
'${quote_sqlite:$local_part}'} {$value}{0}}" in add command: absolute
file name expected for "sqlite" lookup
    111334 added retry item for R:wu-lee@???: errno=-17
more_errno=0 flags=0

This error is another which doesn't make much sense to me, as I'm
passing an absolute file path. Searching online, I found someone else
reporting this error in a thread on this mailing list:

https://www.mail-archive.com/exim-users@exim.org/msg55828.html

Heiko Schlittermann's reply states that the commit for the `file=`
option feature is not part of 4.94. My search the history of the Exim
github repo seems to support that.

Therefore, I gave up and have just set the file globally using the
`sqlite_dbfile` option.  I don't want to use backports or alternative
Exim packages if can avoid it, because that would complicate my server
deployment process considerably.  I can live with this as I don't have
many (any?) other users to support, and I don't currently have any other
uses of sqlite to conflict with.


Regarding Andrew's reply, "so I doubt that [quote_sqlite] is intended to
detaint": I think it would help to be clear in the documentation whether
or not it detaints, specifically here:

> This means that the query cannot use any tainted values, as that
> taints the entire query including the filename - resulting in a
> refusal to open the file.


Presumably lookups, and the sqlite lookup in particular need - by design
- to be able to accept tainted inputs (excepting file paths for
databases), or else they wouldn't be useful tools for detainting data?
In which case either `quote_sqlite` needs to detaint, or the sqlite
lookup needs to allow tainted query strings. (I would hazard a guess the
former is safer.)


A point regarding detainting generally: I've also dropped writing
per-alias logfiles, in favour of writing one combined logfile. This is
because I can't see a way to detaint the `$local_part` variable so it
can be used in the `logfile` option, even if it has been validated to be
safe with a regular expression or similar.

This is not to say that there is no way to detaint `$local_part`, just
that there seems not to be one which would work for my use-case: I need
some sort of wildcard match as I can't predict ahead of time what prefix
an alias can have - there are an essentially an infinite, or at least a
huge number of them, and this is by design.

Which means I can't use a simple list lookup, nor a wildcard lookup, as
these don't support captures, and although I can use the `$sg{}`
expansion to validate the $local_part, I gather it doesn't detaint, so I
can't put the result into the `logfile` parameter.

Therefore, unless I'm mistaken and there is another lookup intended for
this use case, my use-case just isn't possible... Unless I abuse the
lookup detainting, such as in the way described here:

https://jimbobmcgee.wordpress.com/2020/07/29/de-tainting-exim-configuration-variables/

Is it the maintainers' opinion that when tainted text which *can only*
be validated as safe by a wildcard or a regular expression (to use in,
for example, a file path), it should nevertheless still not be possible
to use that to detaint the validated text, in case someone else abuses
this mechanism to create an insecure Exim configuration?


To sum up, for avoidance of (Debian) users tripping over the same things
I did in the future, would the maintainers consider the following? I may
have time for the documentation but I am less confident I could add and
test the code changes.

  * Add examples of all the syntaxes supported by the sqlite lookup
    within the documentation.
  * Likewise, an explanation of which versions of Exim support the new
    `file=` option syntax and which only support `sqlite_dbfile`.
  * Add a warning that the old syntax is obsolete, when used.
  * Clarify the error which seems to suggest the filename, specifically,
    is tainted. (In my case the filename is not the problem so it is
    misleading.)
  * Add support for the new `file=` lookup syntax in the 4.94
    maintenance branch and/or Debian 11 - or if not, at least an error
    which states this syntax is not supported (although is present in
    the documentation)?
  * Add support to Exim for a regular-expression detainting expansion as
    in Perl, in order to support cases like mine which other lookups
    can't handle?


I know the last point has been refused before, but I would like to add
my support for it: detainting text which has been validated safe by a
wildcard or regular-expression seems to me to be a valid and necessary
thing to do. I think not supporting it drives otherwise responsible
users like me towards awkward abuses of lookups and/or away from Exim.

Thanks for the help,


Nick