Af_zz_imgproxy and absolute URL

Tiny Tiny RSS Themes and Plugins
1

Hi,

I tried to use the af_zz_imgproxy enabled for all hosts but it keeps displaying a 404 generated image.
I enabled the debug mode (with &debug=1 at the end of the URL) and I saw the requested URL are no more absolute.

I checked at the code and there is a call to urlencode() when rewriting the URL but no call to urldecode() when trying to relativize the URL and fetch the content. For example, it tries to fetch https://mydomain/http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F04%2Fsamsung-dex-station-2-1000x750.jpg because URL starts with “http%3a%2f%2f” and not “http://”.

So I add a call to urldecode() in the imgproxy function like this:

public function imgproxy() {
$url = rewrite_relative_url(SELF_URL_PATH, urldecode($_REQUEST[“url”]));

And it now works fine.
Could you look at this issue and consider my patch? Thanks all :thumbsup:

2

urldecode is normally not needed in this situation, not sure why this happens on your install

3

Am I the only one in this case? Is there any test you would like I reproduce on my install?
I have not modified it, except to add urldecode as shown above.

4

the test should look more or less like that, i think

<html>
<body>

<p>Parameter: <?php print $_REQUEST["test"]; ?></p>

<?php
   $test_url = htmlspecialchars("?test=" . urlencode("http://google.com"));

   print "<a href=\"$test_url\">click me</a>";
?>

</body>
</html>
5

Thanks! I’ll do the test by Thursday and post the result here.
I think you expect to know if I will be redirected to Google or if stay on my host.
Keep posted :wink:

6

@fox So I made the test. I uploaded the file (to test.php) and clicked on the link.
It leads me to mydomain.com/test.php?test=http%3A%2F%2Fgoogle.com and the content of the page was:

Parameter: http://google.com
click me

So effectively, it doesn’t need to decode the URL from $_REQUEST.
Is there any additional test you would like I run?

7

well yes it was entirely obvious that urldecode() in this case is unnecessary

this does prove that nothing is horribly broken on your system but i’m still not sure why would the plugin, which does essentially exactly same thing, needs it

Is there any additional test you would like I run?

i have no idea what would cause that, maybe try disabling other plugins, idk

8

I’m still looking for clue. I add a print_r($_REQUEST) in the debug output of img_proxy:

(
    [op] => pluginhandler
    [plugin] => af_zz_imgproxy
    [pmethod] => imgproxy
    [url] => http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fsurface-laptop-usb-type-c-1-630x351.jpg
    [debug] => 1
    [ttrss_sid_ssl] => xxxx
    [feedTreeSaveStateCookie] => xxxx
)

So the url is clearly not escaped where the documentation says it should be (and you must not decode again, it’s dangerous!) See PHP: urldecode - Manual

I also run some JS in the console of the browser (I tried with Chrome and Firefox) in order to check if the image source are not “double encoded”:

$$('img').each(function(img) { index = img.src.indexOf('imgproxy'); if (index !== -1) console.log(img.src.substring(index)); });

imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fsurface-laptop-usb-type-c-1-630x351.jpg
imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fsurface-pro-4-stockage-630x473.jpg
imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fsurface-laptop-prix-france.jpg
imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fwindows-10s-wallpaper.jpg
imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fimages.frandroid.com%2Fwp-content%2Fuploads%2F2017%2F05%2Fsurface-laptop-4-couleurs.png

But it’s not the case… I also log the rewrite_url_if_needed method of the plugin but the method is not called twice on the same url (or in a previously encoded url).

So I absolutely have no idea why it doesn’t work :frowning2:

9

yeah the real question here is why isn’t the request parameter urldecoded automatically as it should by php runtime.

since you don’t have this behavior on the test script maybe investigate if your settings are different somehow between tt-rss and test.php? put test.php in the same location as tt-rss or something. which httpd are you using anyway?

also try putting this frandroid url to the test script, see if the results are any different. tbh i’m out of ideas here.

e: my only guess would be some elaborate httpd rewrite rule screwing things up by double-encoding parameters before they are passed to php, you didn’t elaborate on your tt-rss setup tho so idk

10

It seems I found something… I’m using TLS and https with my domain. I checked the configuration I have:

define('SELF_URL_PATH', 'http://mydomain.com/');

And I also have a set of Apache rules to redirect to https:

# HTTPS redirection
RewriteCond %{HTTPS} off
RewriteRule ^(.*)$ https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

So when I looked at network traffic, I see:
http://mydomain.com/public.php?op=pluginhandler&plugin=af_zz_imgproxy&pmethod=imgproxy&url=http%3A%2F%2Fmydomain.com%2Fpublic.php%3Fop%3Dcached_url%26hash%3D4e73b95154f9b51a4971dbe377f7d0c3567454d9.png → 301 redirect to: https://mydomain.com/public.php?op=pluginhandler&plugin=af_zz_imgproxy&pmethod=imgproxy&url=http%253A%252F%252Fmydomain.com%252Fpublic.php%253Fop%253Dcached_url%2526hash%253D4e73b95154f9b51a4971dbe377f7d0c3567454d9.png.
It’s hard to read but the parameters was double escaped during the redirection! :upside_down:
(http%3A%2F%2Fmydomain.com%2F becomes http%253A%252F%252Fmydomain.com%252F)

So the issue was here :v: I’ll fix the host configuration of tt-rss it-self to use https protocol and I’ll add the missing flag NE (noescape) for the rewrite rule: RewriteRule Flags - Apache HTTP Server Version 2.4
Thanks you a lot for your help! :hugging:

11

i think having SELF_URL_PATH refer to http:// while accessing tt-rss over SSL should, at least, produce a warning because it seems to be a very common thing as a result of http to https upgrades which, while not immediately obvious, might cause subtle issues like discussed above.

unfortunately there’s no easily accessible warning system for sanity checking so i’m going to implement this as a hard restriction.

12

What about writing a system log when a user log in with https whereas tt-rss configuration is set on http only?
I don’t know if admin ofter read application logs but I could be an easy solution.

13

it would be really hard to connect this kind of log message to double-escaping of urlencoded parameters, even if we assume people would check logs when things are seemingly (mostly) fine

maybe later i’ll cook up some warning system with a ! button of some kind appearing in the UI somewhere, for now it’s fine imo

e: now that i think about it i think i recall a few more reported issues that were caused by fucked up http->https redirects so maybe hard-blocking this isn’t such a bad idea