NPE in DefaultConversionContext.immutableProperties at line 82

Hello,
There is a bug in Confluence Server 7.7+ in DefaultConversionContext#immutableProperties at line 82.

public ImmutableMap<String, Object> immutableProperties() {
      final ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
      for (Map.Entry<String, Object> entry : properties.entrySet()) {
          final Object value = entry.getValue();
          // simple impl, grab primitives and strings
          if (value instanceof String || Primitives.isWrapperType(value.getClass())) {    // NPE HERE !!!!
              builder.put(entry);
          }
      }
      return builder.build();
  }

The bug is that there is no null check for the value var in this statement:

if (value instanceof String || Primitives.isWrapperType(value.getClass())) 

The value can be (and is, sometimes) null. In that case the instanceof check is false so it tries value.getClass() and blows up w/ an NPE.

Can someone from Atlassian fix this right away?

I’m investigating right now and don’t yet know why there is a null value in the map but IMO that doesn’t matter… the Map can hold null values so null checks are necessary for any code that uses the values.

I have not seen the problem until 7.7+ so maybe something fairly recently added null values to the map. The lack of a null check has been around since well before 7.7. Who knows… probably doesn’t matter… null check map values.

java.lang.NullPointerException
	at com.atlassian.confluence.content.render.xhtml.DefaultConversionContext.immutableProperties(DefaultConversionContext.java:82)
	at <my macro>

Thanks.

2 Likes

oh… and I know we probably shouldn’t be using DefaultConversionContext but there is no API for copying context properties from one ConversionContext to another.

ConversionContext is the currency used by com.atlassian.confluence.macro.Macro#execute so there are use-cases for doing things w/ that ConversionContext that necessitate creating one. In my case, I just need to copy the context properties from my macro’s ConversionContext into a new one. Can’t do it because there is a null value in the ConversionContext that Confluence handed to my macro’s execute() method and when I ask it for its properties it blows up.

1 Like

Root cause is that the ConversionContext that Confluence is feeding to my macro’s execute() method has a property with key=“details_summary_depth” with value=null. Well, that and the fact that DefaultConversionContext doesn’t null check values in the properties map.

DefaultConversionContext.immutableProperties fails w/ NPE on the null value.

To reproduce this I put a Confluence “Page Properties Report” macro on the page anywhere before my macro. That macro screws up the ConversionContext by putting a null property in there that is not properly handled by DefaultConversionContext. If my macro is first on the page it is fine.

This could be exploitable. An evil macro can make life hard by putting null values into the ConversionContext properties map. Can someone from Atlassian please fix this ASAP?

details_summary_depth

1 Like

The Page Properties Report macro was refactored for Confluence 7.7:
https://confluence.atlassian.com/doc/confluence-7-7-release-notes-1004960930.html#Confluence7.7ReleaseNotes-ImprovedPagePropertiesReportmacro

That is probably when the null context property value was introduced.

2 Likes