Azure / azure-notificationhubs-java-backend

Azure Notification Hubs SDK for Java
https://docs.microsoft.com/en-us/azure/notification-hubs/
Apache License 2.0
35 stars 49 forks source link

XML Injection for all registrations #49

Closed jvbeck closed 3 years ago

jvbeck commented 4 years ago

All Registration XML strings are created using string manipulation. It is possible to create tag values and PNS handle values that result in XML that is not what was actually intended. The XML should be generated using an XMLStreamWriter.

marstr commented 4 years ago

Howdy @jvbeck,

You must be looking at code like the following:

https://github.com/Azure/azure-notificationhubs-java-backend/blob/f4120c04bc4cdd53def1002c2bfec7b84ecdf2ea/NotificationHubs/src/com/windowsazure/messaging/Registration.java#L103-L115

Are you experiencing any trouble derived from this, or are you just noting a potential improvement?

jvbeck commented 4 years ago

Hi, I am also referring to code like

https://github.com/Azure/azure-notificationhubs-java-backend/blob/master/NotificationHubs/src/com/windowsazure/messaging/AppleRegistration.java#L69-L78

This sample occurs in every Registration class.

If the NH registration is done by the back-end then the PNS handle must be transferred from the mobile device to the back-end.

I do not know how much mischief could be caused by this XML injection. I merely note that it is possible.

I can easily workaround the problem by overriding the getXml method in "fix" classes that extend the provided Registration classes.

jvbeck commented 4 years ago

Note also my comment on #44

mpodwysocki commented 3 years ago

@jvbeck After review, our backend is not vulnerable to XML Injection attacks but will make a note of this for future versions.