]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/bugs/removal_of_transient_pages.mdwn
Exclude working directory from library path (CVE-2016-1238)
[git.ikiwiki.info.git] / doc / bugs / removal_of_transient_pages.mdwn
index 67fa886b97974adc7adeaf0fe5399e684992f421..6d0caf42eeca6592fe4994f6e103e4d3bcbcd5c9 100644 (file)
@@ -17,3 +17,62 @@ pages, until this is fixed.  --[[Joey]]
 >> --[[Joey]] 
 
 >>> Yes. It's not entirely clear to me why that's there... --s
 >> --[[Joey]] 
 
 >>> Yes. It's not entirely clear to me why that's there... --s
+
+>>>> Yeah, 2461ce0de6231bfeea4d98c86806cdbb85683297 doesn't really
+>>>> say, and I tend to assume that when I've written paranoid code
+>>>> it's there for a reason. I think that here the concern was that
+>>>> the file might be in some underlay that the user should not be able
+>>>> to affect by web edits. The `-f` check seems rather redundant,
+>>>> surely if it's in `%pagesources` ikiwiki has already verified it's
+>>>> safe. --[[Joey]] 
+
+----
+
+[[!template id=gitbranch branch=smcv/ready/transient-rm author="[[Simon McVittie|smcv]]"]]
+
+Here's a branch. It special-cases the `$transientdir`, but in such a way
+that the special case could easily be extended to other locations where
+deletion should be allowed.
+
+It also changes `IkiWiki::prune()` to optionally stop pruning empty
+parent directories at the point where you'd expect it to (for instance,
+previously it would remove the `$transientdir` itself, if it turns out
+to be empty), and updates callers.
+
+The new `prune` API looks like this:
+
+    IkiWiki::prune("$config{srcdir}/$file", $config{srcdir});
+
+with the second argument optional. I wonder whether it ought to look
+more like `writefile`:
+
+    IkiWiki::prune($config{srcdir}, $file);
+
+although that would be either an incompatible change to internal API
+(forcing all callers to update to 2-argument), or being a bit
+inconsistent between the one-and two-argument forms. Thoughts?
+
+--[[smcv]]
+
+> I've applied the branch as-is, so this bug is [[done]].
+> `prune` is not an exported API so changing it would be ok.. 
+> I think required 2-argument would be better, but have not checked
+> all the call sites to see if the `$file` is available split out
+> as that would need. --[[Joey]] 
+
+[[!template id=gitbranch branch=smcv/ready/prune author="[[Simon McVittie|smcv]]"]]
+
+>> Try this, then? I had to make some changes to `attachment`
+>> to make the split versions available. I suggest reviewing
+>> patch-by-patch.
+
+>>> Branch updated; I'd missed a use of prune in ikiwiki.in itself.
+>>> Unfortunately, this means it does still need to support the
+>>> "undefined top directory" case: there isn't an obvious top
+>>> directory for wrappers. --[[smcv]]
+
+>> I also tried to fix a related bug which I found while testing it:
+>> the special case for renaming held attachments didn't seem to work.
+>> (`smcv/wip/rename-held`.) Unfortunately, it seems that with that
+>> change, the held attachment is committed to the `srcdir` when you
+>> rename it, which doesn't seem to be the intention either? --[[smcv]]