alekc / af_refspoof

GNU General Public License v2.0
17 stars 6 forks source link

Doesn't work with semi-modern TTRSS #5

Open mtfurlan opened 3 years ago

mtfurlan commented 3 years ago

Got a bunch of issues trying to use this on ttrss from six months ago (324aef9f6f80e7d1b34f1af2487ab03eebc66f47)

I fixed some of it with this:

diff --git a/init.php b/init.php
index 7f911c1..ae66293 100644
--- a/init.php
+++ b/init.php
@@ -22,7 +22,7 @@ class af_refspoof extends Plugin {
         require_once ("PhCURL.php");

         $this->host = $host;
-        $this->dbh = Db::get();
+        $this->dbh = Db::pdo();
         $host->add_hook($host::HOOK_RENDER_ARTICLE_CDM, $this);
         $host->add_hook($host::HOOK_PREFS_TAB, $this);
     }
@@ -54,14 +54,9 @@ EOF;
             <script type="dojo/method" event="onSubmit" args="evt">
                 evt.preventDefault();
                 if (this.validate()) {
-                    new Ajax.Request('backend.php', {
-                        parameters: dojo.objectToQuery(this.getValues()),
-                        onComplete: function(transport) {
-                            if (transport.responseText.indexOf('error')>=0)
-                                notify_error(transport.responseText);
-                            else notify_info(transport.responseText);
-                        }
-                    });
+                    xhr.post("backend.php", this.getValues(), (reply) => {
+                        Notify.info(reply);
+                    })
                 }
                 </script>
             <table>
@@ -118,7 +113,9 @@ EOF;

                         $entry->setAttribute("srcset", $srcSet);
                     }
-                    $url = $backendURL . '&url=' . urlencode($origSrc) . '&ref=' . urlencode($article['link']);
+                    //$url = $backendURL . '&url=' . urlencode($origSrc) . '&ref=' . urlencode($article['link']);
+                    //TODO: this might be unnecessary
+                    $url = $_SERVER['REQUEST_SCHEME'] . "://" . $_SERVER['SERVER_NAME'] . "/" . $backendURL . '&url=' . urlencode($origSrc) . '&ref=' . urlencode($article['link']);
                     $entry->setAttribute("src",$url);
                 }
                 $article["content"] = $doc->saveXML();
@@ -157,11 +154,13 @@ EOF;
     protected function getFeeds()
     {
         $feeds = array();
-        $result = $this->dbh->query("SELECT id, title
+        $stmt = $this->dbh->prepare("SELECT id, title
                 FROM ttrss_feeds
-                WHERE owner_uid = ".$_SESSION["uid"].
-                " ORDER BY order_id, title");
-        while ($line = $this->dbh->fetch_assoc($result)) {
+                WHERE owner_uid = :sessionID
+                ORDER BY order_id, title");
+        $stmt->execute(['sessionID' => $_SESSION["uid"]]);
+        $result = $stmt->fetch();
+        while ($line = $stmt->fetch(PDO::FETCH_ASSOC)) {
             $feeds[] = (object) $line;
         }
         return $feeds;

But now I just get a CSRF error when calling the redirect, and I ran out of time to poke at it today. I'll keep this issue updated with any progress I may make next time I try to get this working.

mtfurlan commented 3 years ago

This makes it work: http://ix.io/3zhC But that doesn't do any kind csrf protection, which I don't like too much.

If we modify TTRSS a little (I'm now working on current ttrss master, but I don't think anything relevant changed)

diff --git a/classes/pluginhandler.php b/classes/pluginhandler.php
index 75b823822..51d6df9c2 100644
--- a/classes/pluginhandler.php
+++ b/classes/pluginhandler.php
@@ -7,7 +7,7 @@ class PluginHandler extends Handler_Protected {
    function catchall($method) {
        $plugin_name = clean($_REQUEST["plugin"]);
        $plugin = PluginHost::getInstance()->get_plugin($plugin_name);
-       $csrf_token = ($_POST["csrf_token"] ?? "");
+       $csrf_token = ($_REQUEST["csrf_token"] ?? "");

        if ($plugin) {
            if (method_exists($plugin, $method)) {

We can kinda add CSRF stuff to the URL

diff --git a/init.php b/init.php
index 2e0f3ed..45171bf 100644
--- a/init.php
+++ b/init.php
@@ -93,7 +93,8 @@ EOF;
         return "${_SERVER['REQUEST_SCHEME']}://${_SERVER['SERVER_NAME']}"
             . "/backend.php?op=pluginhandler&method=redirect&plugin=af_refspoof"
             . "&refspoof_url=" . urlencode($url)
-            . '&refspoof_ref=' . urlencode($ref);
+            . '&refspoof_ref=' . urlencode($ref)
+            . "&csrf_token=${_SESSION["csrf_token"]}";
     }

     function hook_render_article_cdm($article)

I suspect there is a reason that CSRF stuff isn't put in the URL, so I'll go talk to the TTRSS folks and see what they suggest.

I removed the PhCURL because it was throwing some errors and I didn't care to trace why. Some of the default settings you had might be important though, so I may still try to figure out why and put that back in, it looks a lot cleaner to use than the default php curl library...

Edit: https://github.com/mtfurlan/af_refspoof/tree/fix

alekc commented 3 years ago

Thx @mtfurlan , god this is really old project of mine, I think I've stopped using ttrss about the same time as well.

mtfurlan commented 3 years ago

Would you like me to keep updating here or should I fork refspoof and maintain it there?

Apparently putting CSRF in the URL is bad for a few reasons, and I was suggested to see how https://git.tt-rss.org/fox/ttrss-api-resize does it. Looks like it's caching the images locally.

alekc commented 3 years ago

This project is somewhat still referenced around, so it would probably be better if the development continue to be here. If you want to, I can promote you to a maintainer, so you will be able to deal with PRs and merges