PmWiki | PITS / 01104

Summary: Protection of per-group attachments is done per-page instead of per-group

Created: 2009-06-29 08:55

Status: Closed, added for 2.2.3

Category: Bug

From: Eemeli Aro

Assigned:

Priority: 55

Version: 2.2.2

OS:

Description: Attachments are by default stored per group, and accessible via every page of that group. Hence an attachement is as "secure" as the lowest level of protection any page in the group: if the group has non-protected pages, the attached file could possibly be downloaded without a password.

This has been noted on http://www.pmichaud.com/pipermail/pmwiki-devel/2008-February/001277.html and Cookbook.SecureAttachments.

To fix, I propose the use of a new global variable, $EnableProtectUploadsByGroup that should default to 0 or false, but if enabled, should alter the upload.php attachment read and write functions to read the target page's $Group.GroupAttributes page instead of the page itself.

My own implementation of this is the following set of modifications to upload.php:

--- upload.php	2009-06-03 12:32:06.000000000 +0300
+++ upload.php	2009-06-29 16:51:54.000000000 +0300
@@ -143,11 +143,15 @@
 }

 function HandleUpload($pagename, $auth = 'upload') {
-  global $FmtV,$UploadExtMax,
-    $HandleUploadFmt,$PageStartFmt,$PageEndFmt,$PageUploadFmt;
-  $page = RetrieveAuthPage($pagename, $auth, true, READPAGE_CURRENT);
+  global $FmtV, $UploadExtMax, $HandleUploadFmt, $PageStartFmt, $PageEndFmt,
+    $PageUploadFmt, $GroupAttributesFmt, $EnableProtectUploadsByGroup;
+  if (IsEnabled($EnableProtectUploadsByGroup,0)) {
+    SDV($GroupAttributesFmt,'$Group/GroupAttributes');
+    $pn_upload = FmtPageName($GroupAttributesFmt, $pagename);
+  } else $pn_upload = $pagename;
+  $page = RetrieveAuthPage($pn_upload, $auth, true, READPAGE_CURRENT);
   if (!$page) Abort("?cannot upload to $pagename");
-  PCache($pagename,$page);
+  PCache($pn_upload,$page);
   $FmtV['$UploadName'] = MakeUploadName($pagename,@$_REQUEST['upname']);
   $upresult = htmlspecialchars(@$_REQUEST['upresult']);
   $uprname = htmlspecialchars(@$_REQUEST['uprname']);
@@ -160,8 +164,13 @@
 }

 function HandleDownload($pagename, $auth = 'read') {
-  global $UploadFileFmt, $UploadExts, $DownloadDisposition;
+  global $UploadFileFmt, $UploadExts, $DownloadDisposition,
+    $GroupAttributesFmt, $EnableProtectUploadsByGroup;
   SDV($DownloadDisposition, "inline");
+  if (IsEnabled($EnableProtectUploadsByGroup,0)) {
+    SDV($GroupAttributesFmt,'$Group/GroupAttributes');
+    $pagename = FmtPageName($GroupAttributesFmt, $pagename);
+  }
   $page = RetrieveAuthPage($pagename, $auth, true, READPAGE_CURRENT);
   if (!$page) Abort("?cannot read $pagename");
   $upname = MakeUploadName($pagename, @$_REQUEST['upname']);
@@ -186,8 +195,13 @@

 function HandlePostUpload($pagename, $auth = 'upload') {
   global $UploadVerifyFunction, $UploadFileFmt, $LastModFile, 
-    $EnableUploadVersions, $Now;
-  $page = RetrieveAuthPage($pagename, $auth, true, READPAGE_CURRENT);
+    $EnableUploadVersions, $Now, $GroupAttributesFmt,
+    $EnableProtectUploadsByGroup;
+  if (IsEnabled($EnableProtectUploadsByGroup,0)) {
+    SDV($GroupAttributesFmt,'$Group/GroupAttributes');
+    $pn_upload = FmtPageName($GroupAttributesFmt, $pagename);
+  } else $pn_upload = $pagename;
+  $page = RetrieveAuthPage($pn_upload, $auth, true, READPAGE_CURRENT);
   if (!$page) Abort("?cannot upload to $pagename");
   $uploadfile = $_FILES['uploadfile'];
   $upname = $_REQUEST['upname'];