Bitcoin Forum
July 25, 2024, 09:19:12 PM *
News: Latest Bitcoin Core release: 27.0 [Torrent]
 
   Home   Help Search Login Register More  
Pages: [1]
  Print  
Author Topic: Shrink/expand header persistence (SMF patch)  (Read 178 times)
PowerGlove (OP)
Hero Member
*****
hacker
Offline Offline

Activity: 550
Merit: 4757



View Profile
July 18, 2024, 06:14:49 AM
Merited by LoyceV (12), dkbit98 (10), ABCbits (8), d5000 (5), Xal0lex (5), tranthidung (5), hosseinimr93 (4), hd49728 (2), un_rank (2), Cyrus (1)
 #1

This one was suggested by dkbit98. (Thanks for kicking off the suggestions thread with an easy one; being able to quickly fix the first thing suggested makes me look badass, yo.) Cheesy

The area at the very top of most pages looks like this:



That area can be "collapsed" (by clicking on the minus icon next to the date/time) to save some vertical space:



The problem is that the setting isn't being saved/persisted, so you have to keep collapsing it again and again as you navigate the site. What's interesting is that it is being persisted when you're not logged-in... That's weird, right? Why does it only remember to stay collapsed when you're not logged-in?

It turns out that there are two ways in which this setting gets saved: When you're logged-in, it gets saved as a so-called "theme option" via a JavaScript function named smf_setThemeOption that depends on a support endpoint (/index.php?action=jsoption), and when you're not logged-in, it gets saved as a cookie. (At least, that's how it's supposed to work.)

One of those two ways is still working (the cookie way), and the other is currently broken (the theme option way). I can only hazard a guess as to why smf_setThemeOption() no longer works, but I think what probably happened is that theymos disabled the support endpoint for it. Looking at the code for that endpoint, I can see why he might have done that (it's not a well-designed feature; it exposes too much attack surface relative to its usefulness).

So, I can see two ways to fix the problem:

Fix A

Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not).

The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO).

Code:
--- baseline/Themes/default/index.template.php	2008-04-30 18:30:34.000000000 +0000
+++ modified/Themes/default/index.template.php 2024-07-18 02:47:24.000000000 +0000
@@ -45,24 +45,27 @@
  /* The version this template/theme is for.
  This should probably be the version of SMF it was created for. */
  $settings['theme_version'] = '1.1';
 
  /* Set a setting that tells the theme that it can render the tabs. */
  $settings['use_tabs'] = true;
 
  /* Use plain buttons - as oppossed to text buttons? */
  $settings['use_buttons'] = true;
 
  /* Show sticky and lock status seperate from topic icons? */
  $settings['seperate_sticky_lock'] = true;
+
+ /* Always use cookies to shrink/expand the headers? (Rather than cookies when not logged-in, and theme options otherwise.) */
+ $settings['header_collapse_by_cookie'] = true;
 }
 
 // The main sub template above the content.
 function template_main_above()
 {
  global $context, $settings, $options, $scripturl, $txt, $modSettings;
 
  // Show right to left and the character set for ease of translating.
  echo '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
 <html xmlns="http://www.w3.org/1999/xhtml"', $context['right_to_left'] ? ' dir="rtl"' : '', '><head>
  <meta http-equiv="Content-Type" content="text/html; charset=', $context['character_set'], '" />
  <meta name="description" content="', $context['page_title'], '" />', empty($context['robot_no_index']) ? '' : '
@@ -105,66 +108,66 @@
  // If we're viewing a topic, these should be the previous and next topics, respectively.
  if (!empty($context['current_topic']))
  echo '
  <link rel="prev" href="', $scripturl, '?topic=', $context['current_topic'], '.0;prev_next=prev" />
  <link rel="next" href="', $scripturl, '?topic=', $context['current_topic'], '.0;prev_next=next" />';
 
  // If we're in a board, or a topic for that matter, the index will be the board's index.
  if (!empty($context['current_board']))
  echo '
  <link rel="index" href="' . $scripturl . '?board=' . $context['current_board'] . '.0" />';
 
  // We'll have to use the cookie to remember the header...
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
  {
  $options['collapse_header'] = !empty($_COOKIE['upshrink']);
  $options['collapse_header_ic'] = !empty($_COOKIE['upshrinkIC']);
  }
 
  // Output any remaining HTML headers. (from mods, maybe?)
  echo $context['html_headers'], '
 
  <script language="JavaScript" type="text/javascript"><!-- // --><![CDATA[
  var current_header = ', empty($options['collapse_header']) ? 'false' : 'true', ';
 
  function shrinkHeader(mode)
  {';
 
  // Guests don't have theme options!!
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
  echo '
  document.cookie = "upshrink=" + (mode ? 1 : 0);';
  else
  echo '
  smf_setThemeOption("collapse_header", mode ? 1 : 0, null, "', $context['session_id'], '");';
 
  echo '
  document.getElementById("upshrink").src = smf_images_url + (mode ? "/upshrink2.gif" : "/upshrink.gif");
 
  document.getElementById("upshrinkHeader").style.display = mode ? "none" : "";
  document.getElementById("upshrinkHeader2").style.display = mode ? "none" : "";
 
  current_header = mode;
  }
  // ]]></script>';
 
  // the routine for the info center upshrink
  echo '
  <script language="JavaScript" type="text/javascript"><!-- // --><![CDATA[
  var current_header_ic = ', empty($options['collapse_header_ic']) ? 'false' : 'true', ';
 
  function shrinkHeaderIC(mode)
  {';
 
- if ($context['user']['is_guest'])
+ if ($context['user']['is_guest'] || $settings['header_collapse_by_cookie'])
  echo '
  document.cookie = "upshrinkIC=" + (mode ? 1 : 0);';
  else
  echo '
  smf_setThemeOption("collapse_header_ic", mode ? 1 : 0, null, "', $context['session_id'], '");';
 
  echo '
  document.getElementById("upshrink_ic").src = smf_images_url + (mode ? "/expand.gif" : "/collapse.gif");
 
  document.getElementById("upshrinkHeaderIC").style.display = mode ? "none" : "";
 
  current_header_ic = mode;

Fix B

The other way to go would be to resurrect the disabled endpoint...

The advantage to fixing it this way is that the state would be properly persisted and would effectively be "remembered" no matter what. There's also a very small traffic-wise advantage to not using cookies for this. The disadvantage is that the forum's attack surface would increase (with the below repair: negligibly, in my estimation, but still a strict increase).

I've hardened the offending endpoint, and the result looks safe to me, but theymos should apply his mind carefully to the if-guard I've added to SetJavaScript() before deciding whether it's sensible to restore the endpoint or not (especially considering that there's an alternative fix).

Code:
--- baseline/Sources/Themes.php	2013-01-18 15:35:17.000000000 +0000
+++ modified/Sources/Themes.php 2024-07-18 02:50:57.000000000 +0000
@@ -1181,24 +1181,28 @@
  $context['sub_template'] = $settings['catch_action']['sub_template'];
 }
 
 // Set an option via javascript.
 function SetJavaScript()
 {
  global $db_prefix, $ID_MEMBER, $settings, $user_info;
 
  // Sorry, guests can't do this.
  if ($user_info['is_guest'])
  obExit(false);
 
+ // Best to enforce the low-consequence subset of functionality that this function actually/currently gets called for...
+ if (empty($_GET['var']) || !in_array($_GET['var'], ['collapse_header', 'collapse_header_ic']) || !isset($_GET['val']) || !in_array($_GET['val'], ['0', '1']) || isset($_GET['th']) || isset($_GET['id']))
+ obExit(false);
+
  // Check the session id.
  checkSession('get');
 
  // This good-for-nothing pixel is being used to keep the session alive.
  if (empty($_GET['var']) || !isset($_GET['val']))
  redirectexit($settings['images_url'] . '/blank.gif');
 
  $reservedVars = array(
  'actual_theme_url',
  'actual_images_url',
  'base_theme_dir',
  'base_theme_url',

(Though I've described the problem only with respect to the collapsible section at the top of the page, there's another one, the so-called "info center" at the bottom of the main page, that these fixes also address.)
NotATether
Legendary
*
Offline Offline

Activity: 1680
Merit: 7074


In memory of o_e_l_e_o


View Profile WWW
July 18, 2024, 07:43:18 AM
 #2

I think making the forum use disabled endpoints is a bad idea, because theymos must have disabled it for security reasons as you know. So I would prefer the first patch. Even though I never collapse the header myself honestly - it's just confusing.

Although I am aware that newer versions of Simple Machines do handle this collapsing properly for logged in users.

PS. Where are you pulling all these PHP files from - are they publicly accessible? And how do you test these things?
joker_josue
Legendary
*
Offline Offline

Activity: 1736
Merit: 4758


**In BTC since 2013**


View Profile WWW
July 18, 2024, 12:50:44 PM
 #3

Fix A

Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not).

The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO).

By the way, I never used this feature much, but for those who use it a lot, the fact that the chosen option is not "memorized" can be annoying.

I believe Fix A is the most appropriate. In addition to being simpler to apply, it will not affect the security of the forum. Furthermore, we are all used to there being times when a certain configuration stored in a cookie disappears, due to reset or use of another device. So this turns out not to be a big inconvenience. Because, as the person uses their browser, this should happen very occasionally.

Congratulations PowerGlove on this patch.  Wink

█████████████████████████
████████▀▀████▀▀█▀▀██████
█████▀████▄▄▄▄████████
███▀███▄███████████████
██▀█████████████████████
█████████████████████████
█████████████████████████
█████████████████████████
██▄███████████████▀▀▄▄███
███▄███▀████████▀███▄████
█████▄████▀▀▀▀████▄██████
████████▄▄████▄▄█████████
█████████████████████████
 
 BitList 
█▀▀▀▀











█▄▄▄▄
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
.
REAL-TIME DATA TRACKING
CURATED BY THE COMMUNITY

.
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
▀▀▀▀█











▄▄▄▄█
 
  List #kycfree Websites   
Smartvirus
Legendary
*
Offline Offline

Activity: 1512
Merit: 1137



View Profile
July 18, 2024, 04:57:48 PM
 #4

Fix A

Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not).

The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO).

By the way, I never used this feature much, but for those who use it a lot, the fact that the chosen option is not "memorized" can be annoying.

I believe Fix A is the most appropriate.
Like you, I haven’t used it either neither do I give it much attention but obviously, there are those who do, users whom eventually needs more space to work with and I understand that.
Safe options are always going to be the best approach to have that which should the patches be considered, I guess “Patch A” would be the more appropriate. An option without risk of attack is better than having the memory comfort with a looming possibility of an attack.

It’s an unpopular belief that many users would login their accounts on just any device. Mostly, users stick to their personal desktop/laptop computers or their mobile device. I don’t randomly pick up just any device to login my account, have reluctantly done that on few occasions but, it’s really that rare and in those moments, you don’t seek much comfort.

Since the patch could be remembered on an already activated device, it’s cool.

.
SPIN

       ▄▄▄██████████▄▄▄
     ▄███████████████████▄
   ▄██████████▀▀███████████▄
   ██████████    ███████████
 ▄██████████      ▀█████████▄
▄██████████        ▀█████████▄
█████████▀▀   ▄▄    ▀▀▀███████
█████████▄▄  ████▄▄███████████
███████▀  ▀▀███▀      ▀███████
▀█████▀          ▄█▄   ▀█████▀
 ▀███▀   ▄▄▄  ▄█████▄   ▀███▀
   ██████████████████▄▄▄███
   ▀██████████████████████▀
     ▀▀████████████████▀▀
        ▀▀▀█████████▀▀▀
.
RIUM
..FAST DEPOSITS .........
..AND WITHDRAWALS..
    ▄▄████████▄▄                        ▄██████▄
  ▄███████▀██████▄                    ▄██████████▄
 ██████ ▀▀ ▄ █████       ██          ▄████████████▄
████████  ▄▀▄ ▀██▀      ▄███       ▄███          ███▄
███████▄  ▀▀▀ ▄██      ▄█████▄    ████████    ███████
███████  ██▀  ▄██     ████████▄   ███▀ ▄▄▄    ▄▄▄▄▀██
█████▄▄  ▀▀▄   ██▄    ▀▀█████▀▀   █████▄▄▄▄▄▄▄▄▄▄▄███
 ██████ █ ▄ ▄█████    ▀▄▄▀▀▀▄▄▀   ████████    ██████▀
  ▀███████████████     ▀█████      ▀██████▄▄▄▄████▀▀
    ▀▀█████████▀         ███         ▀▀████████▀▀
..WHEEL OF..
..FORTUNE...
.WELCOME OFFER .
......200% + 50FS.....
▄███████████████████████▄
█████████████████████████
█████████████████████████
█████████████████▀▀██████
████████████▀▀▀    ██████
███████▀▀▀   ▄▀   ███████
████▄     ▄█▀     ███████
███████▄ █▀      ████████
████████▌▐       ████████
█████████ ▄██▄  █████████
███████████████▄█████████
█████████████████████████
▀███████████████████████▀

.PLAY NOW.
[/ta
dkbit98
Legendary
*
Offline Offline

Activity: 2310
Merit: 7345



View Profile WWW
July 18, 2024, 08:31:41 PM
 #5

This one was suggested by dkbit98. (Thanks for kicking off the suggestions thread with an easy one; being able to quickly fix the first thing suggested makes me look badass, yo.) Cheesy
Thanks PowerGlove, that was super-fast and quicker than I expected Smiley
I am voting for Fix A because I think it is more simple to implement it, but we the forum members probably won't notice much difference if Fix B was selected by theymos.

PS
There are other elements in forum that have similar ''issue'', one of them is footer aka Info Center, but this is more useful than a header.
Footer has Recent Posts and Forum Stats.

█▀▀▀











█▄▄▄
▀▀▀▀▀▀▀▀▀▀▀
e
▄▄▄▄▄▄▄▄▄▄▄
█████████████
████████████▄███
██▐███████▄█████▀
█████████▄████▀
███▐████▄███▀
████▐██████▀
█████▀█████
███████████▄
████████████▄
██▄█████▀█████▄
▄█████████▀█████▀
███████████▀██▀
████▀█████████
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
c.h.
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
▀▀▀█











▄▄▄█
▄██████▄▄▄
█████████████▄▄
███████████████
███████████████
███████████████
███████████████
███░░█████████
███▌▐█████████
█████████████
███████████▀
██████████▀
████████▀
▀██▀▀
PowerGlove (OP)
Hero Member
*****
hacker
Offline Offline

Activity: 550
Merit: 4757



View Profile
Today at 10:23:07 AM
 #6

PS. Where are you pulling all these PHP files from - are they publicly accessible?
Yup, they're publicly accessible... I began this whole SMF-patching arc (back when I did my first) by just downloading the archived sources for 1.1.19 (from here) and then getting them to run in an era-appropriate version of Debian inside a QEMU image.

But, over time, I've upgraded my branch of 1.1.19 to get it working within a more modern/comfortable environment (modern PHP version, modern MySQL version, etc.), which has resulted in hundreds of little modifications.

So, there are three versions of SMF that I need to consider when I put together a new patch (my version, theymos' version, and original 1.1.19). What I do is test things on my version (which includes the hundreds of little changes I mentioned earlier, and sometimes also includes some subset of my other patches), but, because that version includes many changes that were never in 1.1.19 and that theymos likely doesn't have in his version, at least, not in exactly the same places/forms, when I'm finished testing a patch, I carefully re-base it against original 1.1.19 (so that theymos has a constant backdrop to consider my diffs against).

I don't have access to theymos' version of SMF, but, even though I can't see it, it does influence the way I put together my patches. Because I know that theymos will be looking at diffs that he can't merge as-is (because they're not based against his version), I try to keep the number of "patch points" to a minimum and try to place them in spots that I figure are relatively stable between all three versions. If I could see theymos' version, then my patches would probably look a lot different than they do, but, for now, I'm basically stuck in a situation where I have to give theymos "impressionistic" diffs that I'm counting on him to be able to read/interpret and then carefully splice into his version while considering all the hazards that I can't see. (Part of the reason that the 2FA patch was such a challenging piece of work was because it's very security-sensitive code, and finding just the right way to express a high-consequence modification like that, given the constraints I've mentioned, took a great deal of effort/thinking.)

And how do you test these things?
As above, in my own modified version of SMF. I crank all the logging/error-reporting stuff up to paranoid levels, and then basically just manually exercise SMF and make sure that the logs stay clean.

Thanks PowerGlove, that was super-fast and quicker than I expected Smiley
No problem. Thanks for suggesting it. Wink

There are other elements in forum that have similar ''issue'', one of them is footer aka Info Center, but this is more useful than a header.
Haha, you didn't read my whole post, did you? Tongue

See the last line of the OP, quoted below:

(Though I've described the problem only with respect to the collapsible section at the top of the page, there's another one, the so-called "info center" at the bottom of the main page, that these fixes also address.)

@theymos: Looking at the first diff again (Fix A), it's actually kind of goofy to use the $settings array the way I did. Only the code in template_main_above() is affected by the new setting, and I don't see anything else ever needing access to it (the category collapse stuff works in its own distinct way), so I'd probably just base the three changed if-statements on a local variable instead (something like $force_collapse_by_cookie), with a similar comment to the one I left in template_init().
Quickseller
Copper Member
Legendary
*
Offline Offline

Activity: 2954
Merit: 2358


View Profile
Today at 06:06:32 PM
 #7

I am curious to know why smf_setThemeOption() was disabled, what risks disabling this was mitigating, and if there are any alternatives to mitigating these risks.

I would tend to disagree with option A because the settings won't persist across devices/browsers. IMO that is really not the expected behavior of an application that requires users to login.

I am really not familiar with the SMF codebase, so I can't comment on if the restrictions on val, th, and id are sufficient to address whatever underlying root cause that was the reason for disabling smf_setThemeOption() in the first place, but I guess it does at a minimum increase security incrementally.

PS. Where are you pulling all these PHP files from - are they publicly accessible? And how do you test these things?
SMF is open source, but the implementation (with modifications) that bitcointalk.org uses is not. So anyone can easily look at the code for SMF 1.1.19, which will generally match what the forum uses, but any customizations that theymos has implemented are not public.

There are pros and cons to doing it this way. On the one hand, if there is a vulnerability in a modification implemented, it might be difficult for a bad actor to find and exploit the vulnerability. On the other hand, a vulnerability will be more difficult for someone with good intentions to find, which might delay the closing of any potential vulnerabilities that could possibly be exploited undetected for some time. Extensive logging can potentially mitigate this drawback, but only to the extent that you are using something to monitor the logs.
un_rank
Hero Member
*****
Offline Offline

Activity: 798
Merit: 786


- Jay -


View Profile WWW
Today at 07:16:04 PM
 #8

By the way, I never used this feature much, but for those who use it a lot, the fact that the chosen option is not "memorized" can be annoying.
I do not think anyone has used the feature much. It is almost unusable as it is, but if this patch gets implemented, I may switch completely to the collapsed version. Much better privacy when using the forum with people around.
I will only undo it when I want to visit the patrol page or check the latest News update.

Great work as always PowerGlove.

- Jay -

██
██
██
██
██
██
██
██
██
██
██
██
██
... LIVECASINO.io    Play Live Games with up to 20% cashback!...██
██
██
██
██
██
██
██
██
██
██
██
██
BitcoinGirl.Club
Legendary
*
Offline Offline

Activity: 2856
Merit: 2757


Bitcoingirl 2 is downloading 💓


View Profile WWW
Today at 07:16:57 PM
 #9

Even though I never collapse the header myself honestly - it's just confusing.
I knew we had collapsible header but I did not know that moving to another page or refreshing it reset the toggle.

On the other hand, a vulnerability will be more difficult for someone with good intentions to find, which might delay the closing of any potential vulnerabilities that could possibly be exploited undetected for some time.
We had many updates of the original software many times but I don't know if we added a document for it. Without the document when theymos will not be around and someone else will handle the forum will be in great problem to smoothly running the forum.

▄▄███████▄▄
▄██████████████▄
▄██████████████████▄
▄████▀▀▀▀███▀▀▀▀█████▄
▄█████████████▄█▀████▄
███████████▄███████████
██████████▄█▀███████████
██████████▀████████████
▀█████▄█▀█████████████▀
▀████▄▄▄▄███▄▄▄▄████▀
▀██████████████████▀
▀███████████████▀
▀▀███████▀▀
.
 MΞTAWIN  THE FIRST WEB3 CASINO   
.
.. PLAY NOW ..
Pages: [1]
  Print  
 
Jump to:  

Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!