Fork me on GitHub
Subscribe 4

Ticket #1029 (fixed enhancement)

db_update.php: Inline script trips over content security policy

  • Created: 2015-04-07 10:51:30
  • Reported by: wet
  • Assigned to: Franz
  • Milestone: 1.5.9
  • Component: upgrading
  • Priority: normal

db_update.php employs a small inline script to refresh itself and tread through the update stages (https://github.com/fluxbb/fluxbb/blob/m … .php#L1909).

Inline scripts are considered harmful and therefore blocked by default on sites with an active Content Security Policy, as we happen to have on our site (http://forum.textpattern.com).

As a consequence we experience a white screen of death at the first stage of db_update.php with nothing but "invisible" script and meta tags rendered to the client. The refresh never happens as CSP blocks the inlined script.

We'd love to see a different method of stepping through the update stages with better CSP compatibility, and maybe a bit of minimal visible output to show some indication of progress while the update stages occur.

History

chris98 2015-04-07 11:30:17

The same thing is also used in admin_maintenance.php, the page exits with JavaScript to reload at the appropriate position.

header() is probably a better choice.

Franz 2015-04-07 15:21:07

  • Milestone set to 1.5.9.

Nice one. I believe we cannot use header() at that point, right? We've probably already generated output.

chris98 2015-04-07 15:28:59

Good catch - you're right. You might be able to use ob_end_clean() and then header though.

chris98 2015-04-07 15:32:43

I've just tried it with ob_end_clean() on a fresh install (in admin_maintenance.php only) and it works fine.

quy 2015-04-07 16:09:12

@Chris98: Please post code or send pull request. Thanks.

chris98 2015-04-07 16:27:45

Ok, I think I've managed to send a pull request successfully.

Still getting used to github ... wink

quy 2015-04-07 17:01:45

It is a no go for me.

Warning: Cannot modify header information - headers already sent by (output started at \admin_maintenance.php:101) in \admin_maintenance.php on line 124

Your pull request has older commits.

I am not a Git expert either. Normally, I pull/merge to get the latest commits. Create a new branch from it. Make my changes. Push the new branch to my Git account. Send a pull request from it.

chris98 2015-04-07 17:04:26

That is odd, I even turned error reporting on E_ALL to see if there were any errors and I didn't get any in my error log either.

Shall I try a new pull request?

Comment edited 1 times (Diff)

quy 2015-04-07 17:10:11

I would do a new pull request. Franz may suggest differently.

Edit: Since I am experiencing the header issue, a new pull request will not help.

Comment edited 1 times (Diff)

quy 2015-04-07 17:23:55

I get the error at this point:

Processing post 878 …

Processing post 879 …

Warning: Cannot modify header information - headers already sent by (output started at \admin_maintenance.php:101) in \admin_maintenance.php on line 124

Comment edited 1 times (Diff)

chris98 2015-04-07 17:29:59

It just clears the output buffer:

http://php.net/ob_end_clean wrote:

This function discards the contents of the topmost output buffer and turns off this output buffering. If you want to further process the buffer's contents you have to call ob_get_contents() before ob_end_clean() as the buffer contents are discarded when ob_end_clean() is called.

That's what is used when showing the error() function too.

I get the error at this point:

Is this the amount of posts per page you've set or has it passed multiple pages at this point? - it could just be an issue with the last header.

Comment edited 3 times (Diff, Diff 2, Diff 3)

quy 2015-04-07 17:39:33

Posts per cycle: 300
Starting post ID: 40
Empty index: checked

Passed multiple pages.
# of posts: 375157

quy 2015-04-07 19:12:54

Successfully did it with 375157 posts without JavaScript:

exit('<meta http-equiv="refresh" content="0;url=admin_maintenance.php'.$query_str.'" />');

Any downside?

wet: Can you give this a try under Administration > Maintenance > Rebuild search index with the following change in admin_maintenance.php:
find:

exit('<script type="text/javascript">window.location="admin_maintenance.php'.$query_str.'"</script><hr /><p>'.sprintf($lang_admin_maintenance['Javascript redirect failed'], '<a href="admin_maintenance.php'.$query_str.'">'.$lang_admin_maintenance['Click here'].'</a>').'</p>');

replace with:

exit('<meta http-equiv="refresh" content="0;url=admin_maintenance.php'.$query_str.'" />');
Comment edited 2 times (Diff, Diff 2)

Franz 2015-04-08 08:45:31

That's how our redirect pages work, too, right? Can you show an additional link to the user just in case the browser does not execute the redirect?

chris98 2015-04-08 08:59:28

Okay, I've found the cause of the header issue, it wasn't showing on my localhost (still have no idea why) but it was showing on my server.

If you place

ob_start();

before

$per_page = isset($_GET['i_per_page']) ? intval($_GET['i_per_page']) : 0;

then you can use the code I added.

	ob_end_clean();
	ob_start();
	header('Location: admin_maintenance.php'.$query_str);
	exit;

The good news is that it will do what it's meant to, and give no errors, the bad news is that it doesn't output any posts being processed and just keeps redirecting now.

@Franz: would you still like me to make that pull request now I've fixed the header issue?

Comment edited 1 times (Diff)

Franz 2015-04-08 09:04:21

Please make it, we can discuss further improvements (like the meta tag) in the PR...

Comment edited 1 times (Diff)

quy 2015-04-08 12:40:27

'Rebuild completed info'        =>    'Once the process has completed, you will be redirected back to this page. It is highly recommended that you have JavaScript enabled in your browser during rebuilding (for automatic redirect when a cycle has completed). If you are forced to abort the rebuild process, make a note of the last processed post ID and enter that ID+1 in "Starting post ID" when/if you want to continue ("Empty index" must not be selected).',

The 2nd sentence has to be removed too.

Comment edited 1 times (Diff)

quy 2015-10-05 16:19:10

I have not tested this, however, adding this should work.

header("Content-Security-Policy: script-src 'unsafe-inline'");

wet please confirm.

Franz 2015-10-27 09:03:50

@quy: The problem with adding that header is that it would overwrite the one already set on Textpattern's forum.

We'll go with Chris' solution for now. Thanks, everyone!

Franz 2015-10-27 09:06:44

  • Owner set to Franz.
  • Status changed from open to fixed.

Thanks, everyone! I merged the PR and removed the sentence from the language file.

quy 2015-11-03 19:58:58

  • Status changed from fixed to open.

Franz 2015-11-08 20:18:14

  • Status changed from open to fixed.

Commit 04cafe8 to fluxbb master

Fix inline script tripping over content security policy

Remove inline script, and only use HTML meta refresh, with a fallback
link to click on when redirect fails.

Closes #1029.