Heads up: several vulnerabilities fixed

I’ve pushed several security-related fixed to trunk, mostly revolving around af_proxy_http but not limited to it.

I’m not sure if all disclosed issues are currently fixed (this is yet to be confirmed) but most should be so I want you guys to update ASAP and generally keep updating to latest trunk code for a few days just in case.

All credit for finding these vulnerabilities (and disclosing them responsibly) goes to Daniel Neagaru and other employees of DigeeX.

I’m going to add that dealing with them was very enjoyable and information I received related to vulnerability findings and methodology was clear and concise. 5/5 would fix bugs reported by them again. :slight_smile:

Now at: v20.09-77faa5d52

Looks like these latest commits disable/deprecate:

‘define(’_SKIP_SELF_URL_PATH_CHECKS’, true);

Correct?
.

i don’t remember consciously breaking but i don’t normally test against it either, what exactly is broken?

some popup things might need a functioning prefix to open correctly i guess.

Maybe ‘‘define(’_SKIP_SELF_URL_PATH_CHECKS’, true);’ isn’t the issue than?

If so? I guess the issue is with the following setting I was using, which someone suggested here in past posts but I couldn’t find it for reference today, but it was working prior to these latest commits?:

define('SELF_URL_PATH', '.');

The above enabled me to connect to the local server when on the LAN, but also connect via the internet. Now with the latest commits and with the SELF_URL_PATH setting as shown above, opening an article headline brings up a new blank tab. Hovering the cursor on the article in tt-rss shows the correct URL, but like I typed opening the link gives me just a blank page in my browser (tried it in incognito mode as well, no diffl!). btw, logged in as admin and checking the log there are no reported errors.

Setting 'define(‘SELF_URL_PATH’, … ’ to either the server’s LAN address or internet address solves the issue, but I loss the flexibility I had previously. I guess possibly setting up reverse proxy could be the workaround? I’ve been trying to avoid going that route.

tl;dr … Configuring SELF_URL_PATH … to the LAN server address or internet server address works to fix the issue. I can live with that, fwiw.
.

yeah you need to have SELF_URL_PATH pointing to a valid URL, “.” is not going to work.

e: btw some less used popup dialogs might be broken now because of CSRF-related changes, report them if you find them. one i’m aware of is default password nag dialog.

The bookmarklet to subscribe to a feed doesn’t work as it used to, now asks you to enter a url instead of just subscribing…

you should still be able to discover feeds based on a website URL but yeah you’ll need to start with a valid URL first.

It’s specified via feed_url in the query string in the url, but isn’t populated in the form e.g. https://tt-rss-server/public.php?op=subscribe&feed_url=https://community.tt-rss.org, you shouldn’t then need to fill in the box on the page

ah. sure, that makes sense. should be fixed now.

https://git.tt-rss.org/fox/tt-rss/commit/9d3c79498368fa99cfde684c759a1c40825aaaa9

The changes to rewrite_relative_url() breaks URLs with absolute paths. For example:

$url = https://example.com/posts/
$rel_url = /posts/this-is-a-blog-post/

Expected result = https://example.com/posts/this-is-a-blog-post/
Actual result = https://example.com/posts/posts/this-is-a-blog-post/

Because $rel_url begins with a forward-slash, the path of $url should be removed. This is a regression that was introduced in the following commit:

https://git.tt-rss.org/fox/tt-rss/commit/c3d14e1fa54c7dade7b1b7955575e2991396d7ef#diff-43e9eb8f6631fec9c8ef9ba4dfd1db532fdcefc1L1544

i don’t particularly like that whole code block, i think it’s too convoluted for its own good.

maybe we should be less forgiving with people who are dumb enough to put relative URLs in their feeds. it would be a lot easier to assume all relative URLs should start from originating site root, inject website hostname in the URL and keep the query part as is.

if this breaks “…/…/blah/something” in the feed, well, too bad.

I think relative URLs are generally considered bad form. I know when I’m building web sites I avoid it, but I also know web sites do use them, unfortunately.

It’s easier (from a coding perspective) to take the host part of $url and just append path from $rel_url, but there are definitely scenarios that will break URLs under this assumption. I think it’s more of a question of what breaks fewer sites?

I’m not sure how many sites I encounter with ../../post.html versus /post.html. Most sites I see are just using the /post.html style, but I’m just offering one person’s opinion.

I just updated to v20.09-9d3c79498 (docker installation). I’m using combined mode. Whenever I click on the title of an ‘opened’ article I get a warning from Firefox:

The information you have entered on this page will be sent over an insecure connection and could be read by a third party.

Are you sure you want to send this information?

despite using SSL.

If I click continue I get a page with a URL ending in /tt-rss/backend.php
The page just shows {"error":{"code":13,"message":null}}

Is this related to the change?

we’re about to find out :slight_smile:

https://git.tt-rss.org/fox/tt-rss/commit/88c4dc405e04b219ed4ab76603f4cf82c073c140

do you have self url path set to something strange, with checks disabled?

Well, the self url path is using http, but I have nginx on my server setup as a reverse proxy to docker. So I access my server using https. Changing the self url path in config.php to using https results in TT-RSS complaining I should be using http. TT-RSS is working perfectly otherwise.

BTW: changing the self url path in .env does not seem to change the config.php (could be user error :grinning:).

oh boy here we go again, and again, and again

Oops, I’ve fallen into the trap. I’ll go fix my other problem first… Sorry to have bothered you.

I believe this change has broken my ability to use non-default ports ie. whatever.tld:3000/feed