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

[5.2] Harden FormattedTextLogger against object injection attacks #44428

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

SniperSister
Copy link
Contributor

@SniperSister SniperSister commented Nov 8, 2024

Summary of Changes

The current implementation of the FormattedTextLogger class creates a potential code execution vulnerability if either Joomla core itself or a third party extension would have an object injection vulnerability via unserialization of user supplied input. This PR adds an exception message for that very specific case, preventing that such an attack payload would be written.

YES, I'm aware that this is a theoretical b/c break. However, weighting the pros and cons of the current implementation, I think that it's a useful change nonetheless.

Credits:

The general mechanism was reported by Drew Webber / mcdruid

Testing Instructions

Apply patch, create a log message by trying to log in into the administrator site with wrong credentials.

Actual result BEFORE applying this Pull Request

Log file is written

Expected result AFTER applying this Pull Request

Log file is written

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Copy link

@ramalama ramalama left a comment

Choose a reason for hiding this comment

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

Tested successfully as described.
Log Entries before and after identical.

@fgsw
Copy link

fgsw commented Nov 8, 2024

@ramalama can you open https://issues.joomla.org/tracker/joomla-cms/44428 and

  • login with your github-account
  • click button "Test this"
  • mark "Tested successfully"

Now the test count as successfull.

@ramalama
Copy link

ramalama commented Nov 8, 2024

I have tested this item ✅ successfully on 4982fc9

Tested successfully as described.
Log Entries before and after identical.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44428.

@Quy
Copy link
Contributor

Quy commented Nov 10, 2024

The test instruction doesn't appear to run the new method. Please confirm.

@SniperSister
Copy link
Contributor Author

As described, there is no option to execute the method in core. That’s why the purpose of the instructions is to confirm that legitimate use cases of that class are unaffected.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 2ed7d84


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44428.

@Quy
Copy link
Contributor

Quy commented Nov 11, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44428.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 11, 2024
@Hackwar Hackwar merged commit efcabfe into joomla:5.2-dev Nov 14, 2024
0 of 2 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 14, 2024
@Hackwar Hackwar added this to the Joomla! 5.2.2 milestone Nov 14, 2024
@Hackwar
Copy link
Member

Hackwar commented Nov 14, 2024

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants