search

factionsecurity / faction

Pen Test Report Generation and Assessment Collaboration
https://www.factionsecurity.com/
GNU General Public License v2.0
432 stars 31 forks source link

Textbox breaks document generation #61

Closed moerketh closed 2 months ago

moerketh commented 3 months ago

Hi, thanks for building Faction! I've been trying it out and tried to get a modified template in, but can't get it past: https://github.com/factionsecurity/faction/blob/4267f949476db122b06f93f1a8d722fd654f8e6d/src/com/fuse/docx/DocxUtils.java#L1131

Actual result

Generation completes with an error, after which the download fails. Here is the log:

com.openhtmltopdf.match INFO:: Matcher created with 175 selectors
java.lang.IllegalStateException: could not located the paragraph in the specified list!
    at com.google.common.base.Preconditions.checkState(Preconditions.java:508)
    at com.fuse.docx.DocxUtils.updateDocWithExtensions(DocxUtils.java:1131)
    at com.fuse.docx.DocxUtils.generateDocx(DocxUtils.java:379)
    at com.fuse.utils.GenerateReport.generateDocxReport(GenerateReport.java:204)
    at com.fuse.tasks.ReportGenThread.run(ReportGenThread.java:72)
    at com.fuse.tasks.TaskQueueExecutor$1.run(TaskQueueExecutor.java:25)
    at java.base/java.lang.Thread.run(Unknown Source)
Finished Generating Report
Update Notifications
Notifications Sent
java.lang.NullPointerException
    at com.fuse.servlets.getReport.doGet(getReport.java:134)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:529)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:623)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:199)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:144)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:51)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:168)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:144)
    at org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter.doFilter(StrutsPrepareAndExecuteFilter.java:125)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:168)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:144)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:168)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:481)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:130)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
    at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:660)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:346)
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:388)
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:936)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1791)
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
    at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1190)
    at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:63)
    at java.base/java.lang.Thread.run(Unknown Source)

Expected result

Ignoring the presence of textboxes would work for my use case. I don't have any templating placeholders in the textbox.

Reproduction

Reproduction is faily easy, put a textbox with the text "Just a text box" in the default-report-template.docx document and upload it via Templates > Report Designer > Edit Sample Template. Then generate a new report and (attempt to) download it.

Additional information

Running Faction 1.2.6

I changed the error to: Preconditions.checkState(index > -1, "could not located the paragraph " + paragraph + " in the specified list! at " + index);

and got: java.lang.IllegalStateException: could not located the paragraph Just a text box in the specified list! at -1

I'm not sure what would be the proper fix in the code.

summitt commented 3 months ago

Thanks for the detailed report. Would you be able to share your template with me privately at develop [at] factionsecurity [dot] com. Nevermind that last part I think I can reproduce it now.

moerketh commented 3 months ago

Thanks for your response! Sure can, I've shared my reproduction code with you via draft PR.

summitt commented 3 months ago

I'm working on getting a release before my DEFCON presentation and this issue will get resolved in that release. Expect it to be mitigated in the main branch in about 2 weeks.

summitt commented 3 months ago

@moerketh This issue is now fixed in the july24-updates branch. You can see the changes here: https://github.com/factionsecurity/faction/commit/e29514424de8eb74de366b67dd13648644d60198

moerketh commented 3 months ago

@summitt can you open the resulting document? The code now passes the unittest, however I think the resulting document is corrupt or invalid.

summitt commented 3 months ago

Yes. The documents worked fine for me.

summitt commented 3 months ago

I just tested on both the report tester and generated an assessment report. No issues with either. Did you get any errors in the console?

summitt commented 3 months ago

@moerketh I was a little too quick to respond. I had created my own template instead of using yours which was checked in as part of the unit test. When I used your template I did get the corrupted file. Digging into this today. Thanks for reporting it!

summitt commented 3 months ago

@moerketh which version of Word did you edit the doc with or did you use something like Google Docs to edit the docx file?

moerketh commented 3 months ago

@summitt, thanks for your efforts! Not sure why my test would be any different than yours. To create the document, I took the default document from https://github.com/factionsecurity/report_templates and added a textbox to it. I'm using what I think is the latest version of the Microsoft Word desktop application (Microsoft® Word for Microsoft 365 MSO (Version 2406) on Windows 11. Lastly, I used the properties > details dialog on the document to remove document properties before committing.

summitt commented 3 months ago

Thanks. A while ago I encountered this problem but it was because I was using an older version of Word. When I took your doc and resaved it the problem went away on my end which made me think that might be the case again. This helps narrow down what might have caused it.

summitt commented 3 months ago

@moerketh I just updated the docx4j libs and removed some old dependencies and that seemed to fix it on my end but I'm not totally convinced. Would mind trying it on your end using the july24-updates branch?

One thing to note: the TOC should be on its own page. Having the textbox (or any other text or variables) on that page may cause it to be removed by the insertion of the TOC. You can use the textbox on another page w/o issue though.

Hopefully, this mitigates the issue. 🤞🏼

moerketh commented 3 months ago

@summitt awesome, thanks for your time and effort on this! I verified tests and runtime, it's fixed 🎉 Agree the placement of the textbox is awkward :) It getting overridden by the TOC and producing a document without errors is absolutely fine for me.

summitt commented 3 months ago

Awesome! glad it worked on your end. Going to leave the issue open until I push the next release in case anyone else has a similar issue.