Subject: [bug#30386] [PATCH v2 cuirass] database: Prevent SQL injection.

From: [email-masked] (Ludovic Courtès)
Date: Fri, 09 Feb 2018 10:51:23 +0100

Hi Danny,

Apologies for not noticing your message earlier!  I was head-down trying
to get this thing in shape and wasn’t checking for email.  I’ll do
better now on, especially since you know way better than me how to deal
with these database issues.  :-)

Danny Milosavljevic <[email-masked]> skribis:

> +         (stmt-text (format #f "\
> +SELECT Builds.id, Builds.timestamp, Builds.starttime, Builds.stoptime, Builds.log, Builds.status, Builds.derivation,\
> +Derivations.job_name, Derivations.system, Derivations.nix_name,\
> +Specifications.repo_name, Specifications.branch \
> +FROM Builds \
> +INNER JOIN Derivations ON Builds.derivation = Derivations.derivation AND Builds.evaluation = Derivations.evaluation \
> +INNER JOIN Evaluations ON Derivations.evaluation = Evaluations.id \
> +INNER JOIN Specifications ON Evaluations.specification = Specifications.repo_name \
> +WHERE (:id IS NULL OR (:id = Builds.id)) \
> +OR (:project IS NULL OR (:project = Specifications.repo_name)) \
> +OR (:jobset IS NULL OR (:jobset = Specifications.branch)) \
> +OR (:job IS NULL OR (:job = Derivations.job_name)) \
> +OR (:system IS NULL OR (:system = Derivations.system)) \
> +OR (:status IS NULL OR (:status = 'done' AND Builds.status >= 0) OR (:status = 'pending' AND Builds.status < 0))

Do you think we can salvage this bit from your patch?  The NULL
optimization looks good, provided the extra conditions don’t make sqlite
slower.  It might allow us to use ‘sqlite-exec’ directly, and thus
benefit from the binding support in there, as in:

  (sqlite-exec db "… WHERE " id " is NULL or …")

Thoughts?

Thanks!

Ludo’.