Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magento2 FI1 and FI2 #200

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Magento2 FI1 and FI2 #200

wants to merge 4 commits into from

Conversation

mcdruid
Copy link
Contributor

@mcdruid mcdruid commented Nov 26, 2024

No description provided.

$parameters = parent::process_parameters($parameters);
// Remove the .php suffix if it has been specified, as it will be added
// by the application.
$parameters['remote_path'] = preg_replace('#.php$#', '', $parameters['remote_path']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$parameters['remote_path'] = preg_replace('#.php$#', '', $parameters['remote_path']);
$parameters['remote_path'] = preg_replace('#.php$#i', '', $parameters['remote_path']);

$parameters = parent::process_parameters($parameters);
// Remove the prefix and suffix if they have been specified, as they
// will be added by the application.
$parameters['remote_path'] = preg_replace('#(^rsl::|.php$)#', '', $parameters['remote_path']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$parameters['remote_path'] = preg_replace('#(^rsl::|.php$)#', '', $parameters['remote_path']);
$parameters['remote_path'] = preg_replace('#(^rsl::|.php$)#i', '', $parameters['remote_path']);

Are you sure this will remove both the prefix and the suffix if both are present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why you'd ask as it seems like an OR in the pattern so perhaps only one replacement would take place.. however:

php > $filename = 'rsl::foobar.php';
php > $filename = preg_replace('#(^rsl::|.php$)#', '', $filename);
php > print $filename;
foobar

I did have to check to be certain though!

Good suggestion to make the patterns case-insensitive.. but I'm not positive the include would work if the prefix was supplied in the payload in uppercase. Will add the i flag anyway.

@mcdruid
Copy link
Contributor Author

mcdruid commented Nov 26, 2024

Actually if we're going to remove the prefix, we should do that on the file part of the path; I'll tweak the pre-processing shortly.

@mcdruid
Copy link
Contributor Author

mcdruid commented Nov 26, 2024

php > print preg_replace('#(rsl::|.php$)#', '', '/path/to/rsl::foobar.php');
/path/to/foobar

php > print preg_replace('#(rsl::|.php$)#', '', '/rsl::foobar.php');
/foobar

php > print preg_replace('#(rsl::|.php$)#', '', 'rsl::foobar.php');
foobar

I think this does what we want now, and it seems pretty unlikely that rsl:: will appear anywhere else in the path and be removed unintentionally.

$parameters = parent::process_parameters($parameters);
// Remove the prefix and suffix if they have been specified, as they
// will be added by the application.
$parameters['remote_path'] = preg_replace('#(rsl::|.php$)#i', '', $parameters['remote_path']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$parameters['remote_path'] = preg_replace('#(rsl::|.php$)#i', '', $parameters['remote_path']);
$parameters['remote_path'] = preg_replace('#(rsl::|[.]php$)#i', '', $parameters['remote_path']);

Otherwise . will match any char :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted! Thanks.

@mcdruid mcdruid changed the title two FI Gadget Chains for Magento2 Magento2 FI1 and FI2 Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants