PortaMx 2.0 Virgo for SMF 2.1 Beta 2 released

[feline]

A other bug I found on the package manager ..

At final install, a html file is shown to see the install result.

Code Select Expand


<redirect url="$scripturl?action=admin;area=pmx_center" timeout="60000">install/installdone.html</redirect>
That worked in ALL releases .. but NOT in the current github release.

That is a bug I think ... can you fix that please !?

[Kindred]

actually - that seems to be part of the (remove HTML in anything -- which in this case, is definitely a potential security issue) -- so the readme displayed after the installation should be using BBC, not HTML any more.

[Illori]

actually - that seems to be part of the (remove HTML in anything -- which in this case, is definitely a potential security issue) -- so the readme displayed after the installation should be using BBC, not HTML any more.

i dont recall that html has been removed from the readme type stuff in installing packages. if we do want to do it, it should be opened as an issue on github.

i tested a mod that is for 2.0 that has html in the uninstaller, it shows up just fine in 2.1... so html has not been disabled in this area.

[Kindred]

Every single mod I have installed on 2.1 has displayed the readme is text -- including not parsing any HTML, if such is included...

and I was happy -- because it is a potential security hole... I could include malicious code in the mod readme...

[Illori]

there is a readme that can be added after you uninstall the mod, that can still be parsed with html

[feline]

Interesting what i read here 

Can anyon now tell me if a html readme works after install or not?
Can I use a BBC readme ?

It was great, if this  point cleared for me .. thanks

[feline]

Well .. I found why html not worked on redirect (in fact, only text will work here, that is not good !!)

Code Select Expand


// Handle a redirect...
elseif ($action['type'] == 'redirect' && !empty($action['redirect_url']))
{
$context['redirect_url'] = $action['redirect_url'];
$context['redirect_text'] = !empty($action['filename']) && file_exists($packagesdir . '/temp/' . $context['base_path'] . $action['filename']) ? $smcFunc['htmlspecialchars'](file_get_contents($packagesdir . '/temp/' . $context['base_path'] . $action['filename'])) : ($context['uninstalling'] ? $txt['package_uninstall_done'] : $txt['package_installed_done']);
$context['redirect_timeout'] = $action['redirect_timeout'];


The $smcFunc['htmlspecialchars'] will destroy all html tag .. that is ugly and have nothing todo with security ..

But .. if smf mean that, then it was a good option to allow here bbc code (as in the readme BEFORE install).

[Kindred]

actually, that *IS* good.  That is exactly what SHOULD be happening - see my comments above.

[feline]

The result it's (maybe) very ugly .. no line breaks, nothing.
I think, a simple <br /> is no risk .. but the result can be read .. the current situation is ugly .. VERY !!

[feline]

Well .. I looked at that and found s solution, they NOT have any risk, but give the result string more redable structure.

In the file Packages.template.php make the simple change:
find:

Code Select Expand


// If we are going to redirect we have a slightly different agenda.
if (!empty($context['redirect_url']))
{
echo '
', $context['redirect_text'], '<br><br>
<a href="', $context['redirect_url'], '">', $txt['package_installed_redirect_go_now'], '</a> | <a href="', $scripturl, '?action=admin;area=packages;sa=browse">', $txt['package_installed_redirect_cancel'], '</a>';
}


replace width:

Code Select Expand


// If we are going to redirect we have a slightly different agenda.
if (!empty($context['redirect_url']))
{
echo '
', str_replace('\n', '<br>', $context['redirect_text']), '<br><br>
<a href="', $context['redirect_url'], '">', $txt['package_installed_redirect_go_now'], '</a> | <a href="', $scripturl, '?action=admin;area=packages;sa=browse">', $txt['package_installed_redirect_cancel'], '</a>';
}


That will replace all "newline" (\n) to a simple <br> and gives the output a little bit more structure without any risk ..

Thank for reading...

[live627]

I thought parse_bbc() already did that.

EDIT: My bad, I should've seen the code before assuming...

[feline]

  at this point no bbc is parsed .. but that is not a big problem, if SMF apply the small change I posted.
That is no risk, but helps to get a textural read.me a better redable ...

a not from me;
I think, a bbs parse at this point is also no risk, but a very better visuabillity 

[margarett]

IIRC, it should destroy HTML (it's one of the security issues patched between 2.0.8 and 2.0.10, as a evil package could use HTML to perform evil stufzzzz )
And a txt file with BBC should be perfectly parsed but as a readme. I never actually tried a redirect to a page (which isn't an SMF page itself) in a file but, according to the same security principle above, it should not be allowed...

@live627, is it possible to parse a page which isn't even tied to SMF?

[live627]

What do you mean? In a SSI page, you can call the function.

[margarett]

There is no SSI in this page
https://github.com/PortaMx/PortaMx-Virgo-2.0-Beta-2/blob/master/install/installdone.html

(admittedly I'm confused about this because I'm not used to see redirects to HTML pages, just inline text and I haven't really tried it
And this is something we need to sanitize because this presents the same level of "security issue" of the other PackMan sanitized stuff... we sanitized package name, readme, etc etc... So we need to make sure that BBC can be parsed (like in the readme) and that no HTML can be used. I think )

[Kindred]

yes, we do...

[feline]

What we do with this page ...

We insert all action results from the database installer in this file, so the user can see what the installer (php) have made.
I think this is a good option to inform the user, what a mod do ...

The minimum html code we need for this is a simple <br>, because a \n do not work here.
And that can be done, with the simple change I posted ...

[feline]

As I see just ..

Why you have removed the Forum width in Theme settings ?

[Illori]

it can break the responsive theme so it was removed.

[feline]

I just comitted the new release for SMF 2.1 (current Github version) 

see https://github.com/PortaMx/PortaMx-Virgo-2.0-Beta-2/