From 5fb0d38ad8b59bef92655b56cf7145cc979b6dea Mon Sep 17 00:00:00 2001 From: zotlabs Date: Mon, 11 Mar 2019 16:29:12 -0700 Subject: [PATCH 1/5] security updates for multiple xchans --- include/security.php | 248 +++++++++++++++++++++++++++++++++---------- 1 file changed, 189 insertions(+), 59 deletions(-) diff --git a/include/security.php b/include/security.php index 44cd605dc..b9705a0e4 100644 --- a/include/security.php +++ b/include/security.php @@ -306,6 +306,7 @@ function change_channel($change_channel) { * * @return string additional SQL where statement */ + function permissions_sql($owner_id, $remote_observer = null, $table = '') { $local_channel = local_channel(); @@ -316,7 +317,7 @@ function permissions_sql($owner_id, $remote_observer = null, $table = '') { * default permissions - anonymous user */ - if($table) + if ($table) $table .= '.'; $sql = " AND {$table}allow_cid = '' @@ -329,38 +330,63 @@ function permissions_sql($owner_id, $remote_observer = null, $table = '') { * Profile owner - everything is visible */ - if(($local_channel) && ($local_channel == $owner_id)) { - $sql = ''; + if (($local_channel) && ($local_channel == $owner_id)) { + return EMPTY_STR; } /** - * Authenticated visitor. Unless pre-verified, - * check that the contact belongs to this $owner_id - * and load the groups the visitor belongs to. - * If pre-verified, the caller is expected to have already - * done this and passed the groups into this function. + * Authenticated visitor. */ else { + $observer = ((! is_null($remote_observer)) ? $remote_observer : get_observer_hash()); - if($observer) { - $groups = init_groups_visitor($observer); - $gs = '<<>>'; // should be impossible to match + if ($observer) { - if(is_array($groups) && count($groups)) { - foreach($groups as $g) - $gs .= '|<' . $g . '>'; + $sec = get_security_ids($owner_id,$observer); + + // always allow the channel owner, even if authenticated as a visitor + + if ($sec['channel_id']) { + foreach ($sec['channel_id'] as $ch) { + if ($observer === $ch) { + return EMPTY_STR; + } + } + } + + if (is_array($sec['allow_cid']) && count($sec['allow_cid'])) { + $ca = []; + foreach ($sec['allow_cid'] as $c) { + $ca[] = '<' . $c . '>'; + } + $cs = implode('|',$ca); } + else { + $cs = '<<>>'; // should be impossible to match + } + + if (is_array($sec['allow_gid']) && count($sec['allow_gid'])) { + $ga = []; + foreach ($sec['allow_gid'] as $g) { + $ga[] = '<' . $g . '>'; + } + $gs = implode('|',$ga); + } + else { + $gs = '<<>>'; // should be impossible to match + } + $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " AND ( NOT ({$table}deny_cid like '%s' OR {$table}deny_gid $regexop '%s') - AND ( {$table}allow_cid like '%s' OR {$table}allow_gid $regexop '%s' OR ( {$table}allow_cid = '' AND {$table}allow_gid = '') ) + " AND ( NOT ({$table}deny_cid regexop '%s' OR {$table}deny_gid $regexop '%s') + AND ( {$table}allow_cid regexop '%s' OR {$table}allow_gid $regexop '%s' OR ( {$table}allow_cid = '' AND {$table}allow_gid = '') ) ) ", - dbesc(protect_sprintf( '%<' . $observer . '>%')), + dbesc($cs), dbesc($gs), - dbesc(protect_sprintf( '%<' . $observer . '>%')), + dbesc($cs), dbesc($gs) ); } @@ -377,6 +403,7 @@ function permissions_sql($owner_id, $remote_observer = null, $table = '') { * * @return string additional SQL where statement */ + function item_permissions_sql($owner_id, $remote_observer = null) { $local_channel = local_channel(); @@ -398,37 +425,59 @@ function item_permissions_sql($owner_id, $remote_observer = null) { } /** - * Authenticated visitor. Unless pre-verified, - * check that the contact belongs to this $owner_id - * and load the groups the visitor belongs to. - * If pre-verified, the caller is expected to have already - * done this and passed the groups into this function. + * Authenticated visitor. */ else { - $observer = (($remote_observer) ? $remote_observer : get_observer_hash()); - if($observer) { + $observer = (($remote_observer) ? $remote_observer : get_observer_hash()); - $s = scopes_sql($owner_id,$observer); + if($observer) { - $groups = init_groups_visitor($observer); + $scope = scopes_sql($owner_id,$observer); + $sec = get_security_ids($owner_id,$observer); - $gs = '<<>>'; // should be impossible to match + // always allow the channel owner, even if authenticated as a visitor - if(is_array($groups) && count($groups)) { - foreach($groups as $g) - $gs .= '|<' . $g . '>'; + if($sec['channel_id']) { + foreach($sec['channel_id'] as $ch) { + if($observer === $ch) { + return EMPTY_STR; + } + } + } + + if (is_array($sec['allow_cid']) && count($sec['allow_cid'])) { + $ca = []; + foreach ($sec['allow_cid'] as $c) { + $ca[] = '<' . $c . '>'; + } + $cs = implode('|',$ca); } + else { + $cs = '<<>>'; // should be impossible to match + } + + if (is_array($sec['allow_gid']) && count($sec['allow_gid'])) { + $ga = []; + foreach ($sec['allow_gid'] as $g) { + $ga[] = '<' . $g . '>'; + } + $gs = implode('|',$ga); + } + else { + $gs = '<<>>'; // should be impossible to match + } + $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " AND (( NOT (deny_cid like '%s' OR deny_gid $regexop '%s') - AND ( allow_cid like '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0 )) - ) OR ( item_private = 1 $s )) + " AND (( NOT (deny_cid regexop '%s' OR deny_gid $regexop '%s') + AND ( allow_cid regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0 )) + ) OR ( item_private = 1 $scope )) ", - dbesc(protect_sprintf( '%<' . $observer . '>%')), + dbesc($cs), dbesc($gs), - dbesc(protect_sprintf( '%<' . $observer . '>%')), + dbesc($cs), dbesc($gs) ); } @@ -465,40 +514,57 @@ function scopes_sql($uid,$observer) { } - - - - - /** * @param string $observer_hash * * @return string additional SQL where statement */ + function public_permissions_sql($observer_hash) { - $groups = init_groups_visitor($observer_hash); + $owner_id = 0; - $gs = '<<>>'; // should be impossible to match + if ($observer_hash) { + + $sec = get_security_ids($owner_id,$observer_hash); + + if (is_array($sec['allow_cid']) && count($sec['allow_cid'])) { + $ca = []; + foreach ($sec['allow_cid'] as $c) { + $ca[] = '<' . $c . '>'; + } + $cs = implode('|',$ca); + } + else { + $cs = '<<>>'; // should be impossible to match + } + + if (is_array($sec['allow_gid']) && count($sec['allow_gid'])) { + $ga = []; + foreach ($sec['allow_gid'] as $g) { + $ga[] = '<' . $g . '>'; + } + $gs = implode('|',$ga); + } + else { + $gs = '<<>>'; // should be impossible to match + } - if(is_array($groups) && count($groups)) { - foreach($groups as $g) - $gs .= '|<' . $g . '>'; - } - $sql = ''; - if($observer_hash) { $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " OR (( NOT (deny_cid like '%s' OR deny_gid $regexop '%s') - AND ( allow_cid like '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0 ) ) - )) + " AND ( NOT (deny_cid regexop '%s' OR deny_gid $regexop '%s') + AND ( allow_cid regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0) ) + ) ", - dbesc(protect_sprintf( '%<' . $observer_hash . '>%')), + dbesc($cs), dbesc($gs), - dbesc(protect_sprintf( '%<' . $observer_hash . '>%')), + dbesc($cs), dbesc($gs) ); } + else { + $sql = EMPTY_STR; + } return $sql; } @@ -510,7 +576,7 @@ function public_permissions_sql($observer_hash) { * In this implementation, a security token is reusable (if the user submits a form, goes back and resubmits the form, maybe with small changes; * or if the security token is used for ajax-calls that happen several times), but only valid for a certain amout of time (3hours). * The "typename" seperates the security tokens of different types of forms. This could be relevant in the following case: - * A security token is used to protekt a link from CSRF (e.g. the "delete this profile"-link). + * A security token is used to protect a link from CSRF (e.g. the "delete this profile"-link). * If the new page contains by any chance external elements, then the used security token is exposed by the referrer. * Actually, important actions should not be triggered by Links / GET-Requests at all, but somethimes they still are, * so this mechanism brings in some damage control (the attacker would be able to forge a request to a form of this type, but not to forms of other types). @@ -587,8 +653,8 @@ function init_groups_visitor($contact_id) { // private profiles are treated as a virtual group $r = q("SELECT abook_profile from abook where abook_xchan in ( " . protect_sprintf($hashes) . " ) and abook_profile != '' "); - if($r) { - foreach($r as $rv) { + if ($r) { + foreach ($r as $rv) { $groups[] = 'vp.' . $rv['abook_profile']; } } @@ -596,8 +662,8 @@ function init_groups_visitor($contact_id) { // physical groups this identity is a member of $r = q("SELECT hash FROM pgrp left join pgrp_member on pgrp.id = pgrp_member.gid WHERE xchan in ( " . protect_sprintf($hashes) . " ) "); - if($r) { - foreach($r as $rr) + if ($r) { + foreach ($r as $rr) $groups[] = $rr['hash']; } return $groups; @@ -605,6 +671,70 @@ function init_groups_visitor($contact_id) { + +function get_security_ids($channel_id, $ob_hash) { + + $ret = [ + 'channel_id' => [], + 'allow_cid' => [], + 'allow_gid' => [] + ]; + + if($channel_id) { + $ch = q("select channel_hash, portable_id from channel where channel_id = %d", + intval($channel_id) + ); + if($ch) { + $ret['channel_id'][] = $ch[0]['channel_hash']; + $ret['channel_id'][] = $ch[0]['portable_id']; + } + } + + $groups = []; + + $x = q("select * from xchan where xchan_hash = '%s'", + dbesc($ob_hash) + ); + + if ($x) { + + // include xchans for all zot-like networks + + $xchans = q("select xchan_hash from xchan where xchan_hash = '%s' OR ( xchan_guid = '%s' AND xchan_pubkey = '%s' ) ", + dbesc($ob_hash), + dbesc($x[0]['xchan_guid']), + dbesc($x[0]['xchan_pubkey']) + ); + + if ($xchans) { + $ret['allow_cid'] = ids_to_array($xchans,'xchan_hash'); + $hashes = ids_to_querystr($xchans,'xchan_hash',true); + + // private profiles are treated as a virtual group + + $r = q("SELECT abook_profile from abook where abook_xchan in ( " . protect_sprintf($hashes) . " ) and abook_profile != '' "); + if($r) { + foreach ($r as $rv) { + $groups[] = 'vp.' . $rv['abook_profile']; + } + } + + // physical groups this identity is a member of + + $r = q("SELECT hash FROM pgrp left join pgrp_member on pgrp.id = pgrp_member.gid WHERE xchan in ( " . protect_sprintf($hashes) . " ) "); + if($r) { + foreach ($r as $rv) { + $groups[] = $rv['hash']; + } + } + $ret['allow_gid'] = $groups; + } + } + + return $ret; +} + + // This is used to determine which uid have posts which are visible to the logged in user (from the API) for the // public_timeline, and we can use this in a community page by making // $perms = (PERMS_NETWORK|PERMS_PUBLIC) unless logged in. From a9172129d2f537eddcd273255895712718fe9543 Mon Sep 17 00:00:00 2001 From: zotlabs Date: Mon, 11 Mar 2019 22:17:36 -0700 Subject: [PATCH 2/5] check zot6 in import_author_xchan --- include/items.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/items.php b/include/items.php index 51aa81017..9287c81db 100755 --- a/include/items.php +++ b/include/items.php @@ -914,6 +914,15 @@ function import_author_xchan($x) { if(array_key_exists('network',$x) && $x['network'] === 'zot') return $y; + // perform zot6 discovery + + if($x['url']) { + $y = discover_by_webbie($x['url'],'zot6'); + if($y) { + return $y; + } + } + if($x['network'] === 'rss') { $y = import_author_rss($x); } From 11116bdcb77eb9fc62db92fdf87cf2cc1d8e5708 Mon Sep 17 00:00:00 2001 From: zotlabs Date: Tue, 12 Mar 2019 03:40:43 -0700 Subject: [PATCH 3/5] typos --- include/security.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/security.php b/include/security.php index b9705a0e4..38cb72263 100644 --- a/include/security.php +++ b/include/security.php @@ -380,8 +380,8 @@ function permissions_sql($owner_id, $remote_observer = null, $table = '') { $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " AND ( NOT ({$table}deny_cid regexop '%s' OR {$table}deny_gid $regexop '%s') - AND ( {$table}allow_cid regexop '%s' OR {$table}allow_gid $regexop '%s' OR ( {$table}allow_cid = '' AND {$table}allow_gid = '') ) + " AND ( NOT ({$table}deny_cid $regexop '%s' OR {$table}deny_gid $regexop '%s') + AND ( {$table}allow_cid $regexop '%s' OR {$table}allow_gid $regexop '%s' OR ( {$table}allow_cid = '' AND {$table}allow_gid = '') ) ) ", dbesc($cs), @@ -471,8 +471,8 @@ function item_permissions_sql($owner_id, $remote_observer = null) { $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " AND (( NOT (deny_cid regexop '%s' OR deny_gid $regexop '%s') - AND ( allow_cid regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0 )) + " AND (( NOT (deny_cid $regexop '%s' OR deny_gid $regexop '%s') + AND ( allow_cid $regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0 )) ) OR ( item_private = 1 $scope )) ", dbesc($cs), @@ -552,8 +552,8 @@ function public_permissions_sql($observer_hash) { $regexop = db_getfunc('REGEXP'); $sql = sprintf( - " AND ( NOT (deny_cid regexop '%s' OR deny_gid $regexop '%s') - AND ( allow_cid regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0) ) + " AND ( NOT (deny_cid $regexop '%s' OR deny_gid $regexop '%s') + AND ( allow_cid $regexop '%s' OR allow_gid $regexop '%s' OR ( allow_cid = '' AND allow_gid = '' AND item_private = 0) ) ) ", dbesc($cs), @@ -681,12 +681,12 @@ function get_security_ids($channel_id, $ob_hash) { ]; if($channel_id) { - $ch = q("select channel_hash, portable_id from channel where channel_id = %d", + $ch = q("select channel_hash, channel_portable_id from channel where channel_id = %d", intval($channel_id) ); if($ch) { $ret['channel_id'][] = $ch[0]['channel_hash']; - $ret['channel_id'][] = $ch[0]['portable_id']; + $ret['channel_id'][] = $ch[0]['channel_portable_id']; } } From 72384ff2cb28afa74f93a15738fdbd95efe1443b Mon Sep 17 00:00:00 2001 From: zotlabs Date: Tue, 12 Mar 2019 15:17:25 -0700 Subject: [PATCH 4/5] add owner permission checks to AS item fetch --- Zotlabs/Module/Item.php | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/Zotlabs/Module/Item.php b/Zotlabs/Module/Item.php index b247df0fd..980d7308d 100644 --- a/Zotlabs/Module/Item.php +++ b/Zotlabs/Module/Item.php @@ -62,9 +62,44 @@ class Item extends Controller { $sql_extra = item_permissions_sql(0); - $r = q("select * from item where mid = '%s' $item_normal $sql_extra limit 1", - dbesc(z_root() . '/item/' . $item_id) + $r = null; + + + // first see if we have this item owned by the current signer + + $x = q("select * from xchan where xchan_hash = '%s'", + dbesc($sigdata['portable_id']) ); + + if ($x) { + + // include xchans for all zot-like networks - these will have the same guid and public key + + $xchans = q("select xchan_hash from xchan where xchan_hash = '%s' OR ( xchan_guid = '%s' AND xchan_pubkey = '%s' ) ", + dbesc($sigdata['portable_id']), + dbesc($x[0]['xchan_guid']), + dbesc($x[0]['xchan_pubkey']) + ); + + if ($xchans) { + $hashes = ids_to_querystr($xchans,'xchan_hash',true); + $r = q("select * from item where mid = '%s' $item_normal and owner_xchan in ( " . protect_sprintf($hashes) . " ) ", + dbesc(z_root() . '/item/' . $item_id) + ); + } + } + + // then see if we can access it as a visitor + + if (! $r) { + + $r = q("select * from item where mid = '%s' $item_normal $sql_extra limit 1", + dbesc(z_root() . '/item/' . $item_id) + ); + } + + // fetch once more with no extra conditions to see what error condition applies + if(! $r) { From cf5a310286079b22ac3d716c28feae115a59539d Mon Sep 17 00:00:00 2001 From: zotlabs Date: Tue, 12 Mar 2019 16:17:34 -0700 Subject: [PATCH 5/5] rework authenticated item fetches (check ACL on the parent, not on the requested item) --- Zotlabs/Module/Item.php | 115 +++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 65 deletions(-) diff --git a/Zotlabs/Module/Item.php b/Zotlabs/Module/Item.php index 980d7308d..df9a80583 100644 --- a/Zotlabs/Module/Item.php +++ b/Zotlabs/Module/Item.php @@ -40,92 +40,78 @@ class Item extends Controller { function init() { - if(Libzot::is_zot_request()) { + if (Libzot::is_zot_request()) { $conversation = false; $item_id = argv(1); - if(! $item_id) + if (! $item_id) http_status_exit(404, 'Not found'); - $portable_id = EMPTY_STR; + $item_normal = " and item.item_hidden = 0 and item.item_type = 0 and item.item_unpublished = 0 and item.item_delayed = 0 and item.item_blocked = 0 "; + + $i = null; + + // do we have the item (at all)? + + $r = q("select * from item where mid = '%s' $item_normal limit 1", + dbesc(z_root() . '/item/' . $item_id) + ); + + if (! $r) { + http_status_exit(404,'Not found'); + } + + // process an authenticated fetch + $sigdata = HTTPSig::verify(EMPTY_STR); if($sigdata['portable_id'] && $sigdata['header_valid']) { $portable_id = $sigdata['portable_id']; observer_auth($portable_id); + + // first see if we have a copy of this item's parent owned by the current signer + // include xchans for all zot-like networks - these will have the same guid and public key + + $x = q("select * from xchan where xchan_hash = '%s'", + dbesc($sigdata['portable_id']) + ); + + if ($x) { + $xchans = q("select xchan_hash from xchan where xchan_hash = '%s' OR ( xchan_guid = '%s' AND xchan_pubkey = '%s' ) ", + dbesc($sigdata['portable_id']), + dbesc($x[0]['xchan_guid']), + dbesc($x[0]['xchan_pubkey']) + ); + + if ($xchans) { + $hashes = ids_to_querystr($xchans,'xchan_hash',true); + $i = q("select id as item_id from item where mid = '%s' $item_normal and owner_xchan in ( " . protect_sprintf($hashes) . " ) ", + dbesc($r[0]['parent_mid']) + ); + } + } } - $item_normal = " and item.item_hidden = 0 and item.item_type = 0 and item.item_unpublished = 0 and item.item_delayed = 0 and item.item_blocked = 0 "; + // if we don't have a parent id belonging to the signer see if we can obtain one as a visitor that we have permission to access $sql_extra = item_permissions_sql(0); - $r = null; - - - // first see if we have this item owned by the current signer - - $x = q("select * from xchan where xchan_hash = '%s'", - dbesc($sigdata['portable_id']) - ); - - if ($x) { - - // include xchans for all zot-like networks - these will have the same guid and public key - - $xchans = q("select xchan_hash from xchan where xchan_hash = '%s' OR ( xchan_guid = '%s' AND xchan_pubkey = '%s' ) ", - dbesc($sigdata['portable_id']), - dbesc($x[0]['xchan_guid']), - dbesc($x[0]['xchan_pubkey']) - ); - - if ($xchans) { - $hashes = ids_to_querystr($xchans,'xchan_hash',true); - $r = q("select * from item where mid = '%s' $item_normal and owner_xchan in ( " . protect_sprintf($hashes) . " ) ", - dbesc(z_root() . '/item/' . $item_id) - ); - } - } - - // then see if we can access it as a visitor - - if (! $r) { - - $r = q("select * from item where mid = '%s' $item_normal $sql_extra limit 1", - dbesc(z_root() . '/item/' . $item_id) + if (! $i) { + $i = q("select id as item_id from item where mid = '%s' $item_normal $sql_extra limit 1", + dbesc($r[0]['parent_mid']) ); } - // fetch once more with no extra conditions to see what error condition applies - - if(! $r) { - - - $r = q("select * from item where mid = '%s' $item_normal limit 1", - dbesc(z_root() . '/item/' . $item_id) - ); - if($r) { - http_status_exit(403, 'Forbidden'); - } - http_status_exit(404, 'Not found'); + if(! $i) { + http_status_exit(403,'Forbidden'); } - - $items = q("select parent as item_id from item where mid = '%s' and uid = %d $item_normal $sql_extra ", - dbesc($r[0]['parent_mid']), - intval($r[0]['uid']) - ); - if(! $items) { - http_status_exit(404, 'Not found'); - } - - $r = $items; - - $parents_str = ids_to_querystr($r,'item_id'); + $parents_str = ids_to_querystr($i,'item_id'); - $items = q("SELECT item.*, item.id AS item_id FROM item WHERE item.parent IN ( %s ) $item_normal $sql_extra ", + $items = q("SELECT item.*, item.id AS item_id FROM item WHERE item.parent IN ( %s ) $item_normal ", dbesc($parents_str) ); @@ -133,9 +119,8 @@ class Item extends Controller { http_status_exit(404, 'Not found'); } - $r = $items; - xchan_query($r,true); - $items = fetch_post_tags($r,true); + xchan_query($items,true); + $items = fetch_post_tags($items,true); $observer = App::get_observer(); $parent = $items[0];