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

Unexpected javascript injection in browser #18580

Open
2 tasks done
Aeto-J opened this issue Dec 19, 2024 · 8 comments
Open
2 tasks done

Unexpected javascript injection in browser #18580

Aeto-J opened this issue Dec 19, 2024 · 8 comments

Comments

@Aeto-J
Copy link
Contributor

Aeto-J commented Dec 19, 2024

Code of Conduct

  • I agree to follow this project's Code of Conduct

Is there an existing issue for this?

  • I have searched the existing issues

Version

10.0.17

Bug description

Hello,
The issue concerns the hook "show_in_timeline".
This hook can be used to inject Javascript or HTML to modify data in a timeline form for an ITIL object.
But the hook is also used in NotificationTargetCommonITILObject::getDataForObject method via a call to CommonITILObject::getTimelineItems method, when adding or updating an ITIL object.
It means that during the notification process, we are going to inject in the PHP output buffer some Javascript or HTML code (and this code will reach the browser).
We must not inject anything in the browser during notification process.
Thank you,
regards
Aeto-J

Relevant log output

Uncaught ReferenceError: $ is not defined

Page URL

No response

Steps To reproduce

In your plugin add hook "show_in_timeline" for Ticket object to inject javascript with jQuery

echo Html::scriptBlock("
    $(document).ready(function() {
        console.log('in hook');
    })
")

Create a notification on Ticket creation with a template.
Create a Ticket.
Error appears in debug console (Uncaught ReferenceError: $ is not defined)

Your GLPI setup information

No response

Anything else?

No response

@cconard96
Copy link
Contributor

No plugins should be outputting anything during hooks that are only meant to modify the data they are provided, but if they do, it is the plugin author's responsibility to output only safe content.

@cconard96 cconard96 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2024
@cconard96
Copy link
Contributor

In the future if you believe you found a security issue, please follow the GLPI security policy. Using GitHub issues is not responsible disclosure.

https://github.com/glpi-project/glpi/security

@Aeto-J
Copy link
Contributor Author

Aeto-J commented Dec 19, 2024

I'm okay with you, but this hook is not meant to modify data, it is meant to "items display related"
so, it's normal to add display related block of javascript.
see attach screen copy, the description of the show_in_timeline hook

Image

@cedric-anne
Copy link
Member

This hook is meant to inject your HTML/JS code in the timeline, it is the expected behaviour.

@Aeto-J
Copy link
Contributor Author

Aeto-J commented Dec 19, 2024

this is what we are speaking about, we want to inject javascript in timeline but this hook is also called by notification process

@cconard96
Copy link
Contributor

The documentation needs to be clarified.

The hook receives a reference of the array of timeline items and expects plugins to add/remove/change the items in the array. It does not expect you to directly output anything. It is a data hook, not a display hook. If you want to display something in the timeline, use the TIMELINE_ANSWER_ACTIONS hook to register a new timeline itemtype and specify a twig template. Then put what you need to display in that template. Use the SHOW_IN_TIMELINE to add items of that type to the timeline.

If you just want to include a JS script on that specific page, check the requested URL in your plugin's init function and use the ADD_JAVASCRIPT hook.

@Aeto-J
Copy link
Contributor Author

Aeto-J commented Dec 19, 2024

even the name of the hook is not good, it should not be show_something because all the hooks that start with show_ mean that JS/HTML can be output

@cedric-anne cedric-anne reopened this Dec 19, 2024
@Aeto-J
Copy link
Contributor Author

Aeto-J commented Dec 19, 2024

Moreover the hook show_in_timeline is called for other tabs than the processing tab
example: when adding a ticket to a change then the show_in_timeline is called (due to the notification process)

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

No branches or pull requests

3 participants