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

Potential leak of application internals in ExportedObject.getAnnotations() #18

Closed
hal0 opened this issue May 29, 2018 · 5 comments
Closed
Labels

Comments

@hal0
Copy link

hal0 commented May 29, 2018

If a DBusInterface adds any class-level annotations that have a value() method, even if they do not pertain to DBus itself, they will get leaked as introspection data to any clients that care.

In the case of an annotation including information pertaining to implementation details (including crypto suites, hard-coded hostnames, etc.) this information would be freely transmitted to the user.

This is because ExportedObject.getAnnotations() does not filter which annotations it includes in the introspection data:

https://github.com/hypfvieh/dbus-java/blob/master/src/main/java/org/freedesktop/dbus/messages/ExportedObject.java#L56-L67

@hypfvieh
Copy link
Owner

hypfvieh commented Jun 4, 2018

Thanks for the report. I don't have much spare time atm, but I hope the fix I just commited will handle this issue

@hal0
Copy link
Author

hal0 commented Jun 6, 2018

I have not tried it, but 01381b9 appears to break all annotations, as Annotation will never be an instance of DBusInterface. You would have to use annotation.annotationType().isAssignableFrom(DBusInterface.class), which also would never evaluate to true since that would never be the case.

If I may make a recommendation, create a @DBusAnnotation annotation that is used to annotate other annotations that should be considered by the method referred to in 01381b9, which would enumerate each annotation and check if annotation.annotationType().getAnnotation(DBusAnnotation.class) != null, in which case the former annotation would be included in the introspection output.

This creates an explicit API that pushes the security concerns onto the consumer of dbus-java, allowing them to make those security decisions.

Further, as far as I'm aware, annotations within introspection data that don't pertain to D-Bus proper aren't used by any of the standard tooling and thus are rather useless to anyone that doesn't explicitly consume them via the org.freedesktop.DBus.Introspectable interface (see Introspection Format from the spec, which as of this writing outlines only 4 annotations).

Therefore, this could potentially be moot and only the annotations that D-Bus cares about could be checked (i.e. calling iface.getAnnotation(DBusInterfaceName.class) directly). This would be slightly simpler code as well as much more restrictive and thus more secure.

Thank you for the response - the concern is appreciated.

Just a suggestion. Thank you for your response.

@hypfvieh
Copy link
Owner

hypfvieh commented Jun 6, 2018

If you already have a proper fix, please provide a pull request and will merge it.

@hypfvieh
Copy link
Owner

any news on this?
Did you review my changes?

@hypfvieh
Copy link
Owner

as there is no feedback yet, I assume this issue is fixed.

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

No branches or pull requests

2 participants