From c2ea9cc40319675766808d3136cb452c2b7f32d8 Mon Sep 17 00:00:00 2001 From: Madhur Jain <87946372+jmadhur87@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:19:35 +0530 Subject: [PATCH 1/3] chore: Update int to Long_Int (#1081) --- .../cli/cmd/AbstractSSCIssueTemplateUpdateCommand.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/issue_template/cli/cmd/AbstractSSCIssueTemplateUpdateCommand.java b/fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/issue_template/cli/cmd/AbstractSSCIssueTemplateUpdateCommand.java index 1ec99fa9144..ac8a70ae2bb 100644 --- a/fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/issue_template/cli/cmd/AbstractSSCIssueTemplateUpdateCommand.java +++ b/fcli-core/fcli-ssc/src/main/java/com/fortify/cli/ssc/issue_template/cli/cmd/AbstractSSCIssueTemplateUpdateCommand.java @@ -16,7 +16,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.IntNode; +import com.fasterxml.jackson.databind.node.LongNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fortify.cli.common.json.JsonHelper; import com.fortify.cli.common.output.transform.IActionCommandResultSupplier; @@ -63,7 +63,7 @@ protected ArrayNode getCustomTagIds(UnirestInstance unirest, SSCIssueTemplateDes currentTags, addTagsMixin.getTagSpecs(), rmTagsMixin.getTagSpecs() - ).map(tag -> new IntNode(Integer.valueOf(tag.getId()))).collect(JsonHelper.arrayNodeCollector()); + ).map(tag -> new LongNode(Long.valueOf(tag.getId()))).collect(JsonHelper.arrayNodeCollector()); } @Override From 10d00a9afe343afb0bcbe31a01bd979b969c72a7 Mon Sep 17 00:00:00 2001 From: mjain6 Date: Tue, 22 Sep 2026 11:18:27 +0530 Subject: [PATCH 2/3] Address three security findings from FAA scan --- .../fpr/processor/DescriptionProcessor.java | 44 ++++++++++++------- .../writer/record/impl/RecordWriterCsv.java | 19 +++++++- .../cmd/AbstractReportGenerateCommand.java | 13 +----- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/DescriptionProcessor.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/DescriptionProcessor.java index c516d129525..282f8c09dba 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/DescriptionProcessor.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/DescriptionProcessor.java @@ -229,20 +229,20 @@ public String render(Vulnerability vuln, ReplacementData replacementData, boolea private static class ParagraphElement extends FancyElement { private final List altChildren = new ArrayList<>(); - ParagraphElement(String content) { + ParagraphElement(String content, int depth) { String altParagraphTag = ""; int altStart = content.indexOf(altParagraphTag); if (altStart != -1) { String mainContent = content.substring(0, altStart); - this.children.addAll(FvdlParser.parse(mainContent)); + this.children.addAll(FvdlParser.parse(mainContent, depth)); int altEnd = content.indexOf("", altStart); if (altEnd != -1) { String altContent = content.substring(altStart + altParagraphTag.length(), altEnd); - this.altChildren.addAll(FvdlParser.parse(altContent)); + this.altChildren.addAll(FvdlParser.parse(altContent, depth)); } } else { - this.children.addAll(FvdlParser.parse(content)); + this.children.addAll(FvdlParser.parse(content, depth)); } } @@ -269,9 +269,9 @@ public String render(Vulnerability vuln, ReplacementData replacementData, boolea private static class IfDefElement extends FancyElement { private final String var; - IfDefElement(String attributes, String content) { + IfDefElement(String attributes, String content, int depth) { this.var = FvdlParser.parseAttribute("var", attributes); - this.children.addAll(FvdlParser.parse(content)); + this.children.addAll(FvdlParser.parse(content, depth)); } @Override @@ -289,9 +289,9 @@ public String render(Vulnerability vuln, ReplacementData replacementData, boolea private static class IfNotDefElement extends FancyElement { private final String var; - IfNotDefElement(String attributes, String content) { + IfNotDefElement(String attributes, String content, int depth) { this.var = FvdlParser.parseAttribute("var", attributes); - this.children.addAll(FvdlParser.parse(content)); + this.children.addAll(FvdlParser.parse(content, depth)); } @Override @@ -309,9 +309,9 @@ public String render(Vulnerability vuln, ReplacementData replacementData, boolea private static class ConditionalTextElement extends FancyElement { private final String condition; - ConditionalTextElement(String attributes, String content) { + ConditionalTextElement(String attributes, String content, int depth) { this.condition = FvdlParser.parseAttribute("condition", attributes); - this.children.addAll(FvdlParser.parse(content)); + this.children.addAll(FvdlParser.parse(content, depth)); } @Override @@ -340,6 +340,9 @@ private static final class FvdlParser { private static final String REPLACE_TAG = "Replace "; private static final String REPLACE_END_TAG = "/>"; + // Guards against StackOverflowError on deeply nested tags in untrusted FPR input + private static final int MAX_NESTING_DEPTH = 100; + public static String parseAndRender(String text, Vulnerability vuln, ReplacementData data) { if (text == null || text.isEmpty()) { @@ -359,7 +362,16 @@ public static String parseAndRender(String text, Vulnerability vuln, Replacement } public static List parse(String text) { + return parse(text, 0); + } + + public static List parse(String text, int depth) { List elements = new ArrayList<>(); + if (depth >= MAX_NESTING_DEPTH) { + logger.warn("Maximum description nesting depth of {} exceeded; rendering remaining content as plain text", MAX_NESTING_DEPTH); + elements.add(new TextElement(text)); + return elements; + } int cursor = 0; while (cursor < text.length()) { @@ -413,7 +425,7 @@ public static List parse(String text) { String fullAttributes = text.substring(contentStart-1, closingTagStart); String content = text.substring(closingTagStart + 1, contentEnd); - elements.add(createStructuredElement(startTag, fullAttributes, content)); + elements.add(createStructuredElement(startTag, fullAttributes, content, depth + 1)); cursor = contentEnd + endTag.length(); tagFound = true; @@ -433,16 +445,16 @@ public static List parse(String text) { return elements; } - private static Stringable createStructuredElement(String tag, String attributes, String content) { + private static Stringable createStructuredElement(String tag, String attributes, String content, int depth) { switch (tag) { case "Paragraph>": - return new ParagraphElement(content); + return new ParagraphElement(content, depth); case "IfDef ": - return new IfDefElement(attributes, content); + return new IfDefElement(attributes, content, depth); case "IfNotDef ": - return new IfNotDefElement(attributes, content); + return new IfNotDefElement(attributes, content, depth); case "ConditionalText ": - return new ConditionalTextElement(attributes, content); + return new ConditionalTextElement(attributes, content, depth); default: logger.warn("Unknown structured tag type: {}", tag); return new TextElement(""); // Should not happen diff --git a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java index 040bc34fcb5..f153914e112 100644 --- a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java +++ b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java @@ -19,6 +19,7 @@ import com.fasterxml.jackson.core.JsonGenerator.Feature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; import com.fasterxml.jackson.dataformat.csv.CsvFactory; import com.fasterxml.jackson.dataformat.csv.CsvGenerator; import com.fasterxml.jackson.dataformat.csv.CsvSchema; @@ -30,11 +31,27 @@ // TODO This class uses a Jackson generator, so should we extend from AbstractRecordWriterJackson? @RequiredArgsConstructor public class RecordWriterCsv extends AbstractRecordWriter { + // Leading characters that spreadsheet applications interpret as the start of a formula + private static final String FORMULA_TRIGGER_CHARS = "=+-@\t\r\n"; @Getter private final RecordWriterConfig config; @Override protected void append(CsvGenerator out, ObjectNode formattedRecord) throws IOException { - out.writeTree(formattedRecord); + out.writeTree(escapeFormulas(formattedRecord)); + } + + /** Neutralize CSV formula injection by prefixing affected text values with a single quote. */ + private static ObjectNode escapeFormulas(ObjectNode formattedRecord) { + formattedRecord.fields().forEachRemaining(e->{ + var value = e.getValue(); + if ( value!=null && value.isTextual() ) { + var text = value.textValue(); + if ( !text.isEmpty() && FORMULA_TRIGGER_CHARS.indexOf(text.charAt(0))>=0 ) { + e.setValue(new TextNode("'"+text)); + } + } + }); + return formattedRecord; } @Override diff --git a/fcli-core/fcli-license/src/main/java/com/fortify/cli/common/report/cli/cmd/AbstractReportGenerateCommand.java b/fcli-core/fcli-license/src/main/java/com/fortify/cli/common/report/cli/cmd/AbstractReportGenerateCommand.java index 8834da20961..31bc06ff558 100644 --- a/fcli-core/fcli-license/src/main/java/com/fortify/cli/common/report/cli/cmd/AbstractReportGenerateCommand.java +++ b/fcli-core/fcli-license/src/main/java/com/fortify/cli/common/report/cli/cmd/AbstractReportGenerateCommand.java @@ -29,6 +29,7 @@ import com.fortify.cli.common.report.writer.ReportDirWriter; import com.fortify.cli.common.report.writer.ReportZipWriter; import com.fortify.cli.common.util.FcliBuildProperties; +import com.fortify.cli.common.util.FileUtils; import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Mixin; @@ -74,7 +75,7 @@ private IReportWriter createReportWriter() { deleteExisting(outputArgGroup.reportZipName, File::delete); return new ReportZipWriter(outputArgGroup.reportZipName, getCommandHelper().getMessageResolver()); } else if ( StringUtils.isNotBlank(outputArgGroup.reportDirName) ) { - deleteExisting(outputArgGroup.reportDirName, this::deleteDirectory); + deleteExisting(outputArgGroup.reportDirName, f->FileUtils.deleteRecursive(f.toPath())); return new ReportDirWriter(outputArgGroup.reportDirName, getCommandHelper().getMessageResolver()); } else { throw new FcliSimpleException("Either --report-file or --report-dir must be specified"); @@ -89,16 +90,6 @@ private void deleteExisting(String name, Consumer deleter) { } } - private boolean deleteDirectory(File directoryToBeDeleted) { - File[] allContents = directoryToBeDeleted.listFiles(); - if (allContents != null) { - for (File file : allContents) { - deleteDirectory(file); - } - } - return directoryToBeDeleted.delete(); - } - @Override public final boolean isSingular() { return true; From a6cc18db966130893d978ad8f5fc202c877ddf66 Mon Sep 17 00:00:00 2001 From: mjain6 Date: Fri, 25 Sep 2026 14:45:23 +0530 Subject: [PATCH 3/3] Add review comment changes --- .../writer/record/RecordWriterStyle.java | 11 ++- .../writer/record/impl/RecordWriterCsv.java | 82 +++++++++++++------ 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/RecordWriterStyle.java b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/RecordWriterStyle.java index 3e427e14060..c00f637459b 100644 --- a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/RecordWriterStyle.java +++ b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/RecordWriterStyle.java @@ -105,6 +105,11 @@ public final boolean isEnvelope() { return getOrDefault(RecordWriterStyleElementGroup.ENVELOPE)==RecordWriterStyleElement.envelope; } + /** Indicates whether CSV values that spreadsheet applications could interpret as formulas should be escaped. Defaults to csv-escape. */ + public final boolean isCsvEscape() { + return getOrDefault(RecordWriterStyleElementGroup.CSV_ESCAPE)==RecordWriterStyleElement.csv_escape; + } + private final RecordWriterStyleElement getOrDefault(RecordWriterStyleElementGroup group) { return styleElementsByGroup.getOrDefault(group, group.defaultStyle()); } @@ -119,7 +124,8 @@ public static enum RecordWriterStyleElement { md_border(RecordWriterStyleElementGroup.BORDER), wrap(RecordWriterStyleElementGroup.WRAP), no_wrap(RecordWriterStyleElementGroup.WRAP), fast_output(RecordWriterStyleElementGroup.FAST_OUTPUT), no_fast_output(RecordWriterStyleElementGroup.FAST_OUTPUT), - envelope(RecordWriterStyleElementGroup.ENVELOPE), no_envelope(RecordWriterStyleElementGroup.ENVELOPE) + envelope(RecordWriterStyleElementGroup.ENVELOPE), no_envelope(RecordWriterStyleElementGroup.ENVELOPE), + csv_escape(RecordWriterStyleElementGroup.CSV_ESCAPE), no_csv_escape(RecordWriterStyleElementGroup.CSV_ESCAPE) ; @Getter private final RecordWriterStyleElementGroup group; @@ -142,7 +148,8 @@ public static enum RecordWriterStyleElementGroup { BORDER("no-border"), WRAP("wrap"), FAST_OUTPUT("fast-output"), - ENVELOPE("no-envelope"); + ENVELOPE("no-envelope"), + CSV_ESCAPE("csv-escape"); private final String defaultStyleElementName; diff --git a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java index f153914e112..fb79c05e930 100644 --- a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java +++ b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/output/writer/record/impl/RecordWriterCsv.java @@ -16,10 +16,13 @@ import java.io.Writer; import java.util.function.Function; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonGenerator.Feature; +import com.fasterxml.jackson.core.SerializableString; +import com.fasterxml.jackson.core.TreeNode; +import com.fasterxml.jackson.core.util.JsonGeneratorDelegate; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; -import com.fasterxml.jackson.databind.node.TextNode; import com.fasterxml.jackson.dataformat.csv.CsvFactory; import com.fasterxml.jackson.dataformat.csv.CsvGenerator; import com.fasterxml.jackson.dataformat.csv.CsvSchema; @@ -30,28 +33,12 @@ // TODO This class uses a Jackson generator, so should we extend from AbstractRecordWriterJackson? @RequiredArgsConstructor -public class RecordWriterCsv extends AbstractRecordWriter { - // Leading characters that spreadsheet applications interpret as the start of a formula - private static final String FORMULA_TRIGGER_CHARS = "=+-@\t\r\n"; +public class RecordWriterCsv extends AbstractRecordWriter { @Getter private final RecordWriterConfig config; @Override - protected void append(CsvGenerator out, ObjectNode formattedRecord) throws IOException { - out.writeTree(escapeFormulas(formattedRecord)); - } - - /** Neutralize CSV formula injection by prefixing affected text values with a single quote. */ - private static ObjectNode escapeFormulas(ObjectNode formattedRecord) { - formattedRecord.fields().forEachRemaining(e->{ - var value = e.getValue(); - if ( value!=null && value.isTextual() ) { - var text = value.textValue(); - if ( !text.isEmpty() && FORMULA_TRIGGER_CHARS.indexOf(text.charAt(0))>=0 ) { - e.setValue(new TextNode("'"+text)); - } - } - }); - return formattedRecord; + protected void append(JsonGenerator out, ObjectNode formattedRecord) throws IOException { + out.writeTree(formattedRecord); } @Override @@ -61,7 +48,7 @@ protected Function createRecordFormatter(ObjectNode obje } @Override - protected void close(CsvGenerator out) throws IOException { + protected void close(JsonGenerator out) throws IOException { if ( config.getStyle().isArray() ) { out.writeEndArray(); } out.close(); } @@ -72,7 +59,7 @@ protected void closeWithNoData(Writer writer) throws IOException { } @Override - protected CsvGenerator createOut(Writer writer, ObjectNode formattedRecord) throws IOException { + protected JsonGenerator createOut(Writer writer, ObjectNode formattedRecord) throws IOException { if ( formattedRecord==null ) { return null; } CsvSchema.Builder schemaBuilder = CsvSchema.builder(); formattedRecord.fieldNames().forEachRemaining(schemaBuilder::addColumn); @@ -85,6 +72,55 @@ protected CsvGenerator createOut(Writer writer, ObjectNode formattedRecord) thro if ( config.getStyle().isArray() ) { result.writeStartArray(); } - return result; + // Escape formula-triggering values as they're written, without mutating the input ObjectNode + return config.getStyle().isCsvEscape() ? new FormulaEscapingGenerator(result) : result; + } + + /** + * Prevents CSV formula injection (CWE-1236) by prefixing values starting with a formula-trigger + * character with a single quote, applied as Jackson writes each value to the underlying generator. + * Unlike pre-scanning and mutating the record's {@link ObjectNode}, this also covers non-textual + * node values (e.g. {@code POJONode}) that still get serialized as strings, leaves a potentially + * shared {@link ObjectNode} untouched, and avoids a separate pass over record fields. + */ + private static final class FormulaEscapingGenerator extends JsonGeneratorDelegate { + // Leading characters that spreadsheet applications interpret as the start of a formula + private static final String FORMULA_TRIGGER_CHARS = "=+-@\t\r\n"; + + FormulaEscapingGenerator(JsonGenerator delegate) { + super(delegate); + } + + @Override + public void writeTree(TreeNode rootNode) throws IOException { + // Invoke the codec with 'this' as target (JsonGenerator's default writeTree() behavior), + // instead of JsonGeneratorDelegate's default of forwarding to the delegate and bypassing + // the writeString() overrides below. + if ( rootNode==null ) { + writeNull(); + } else { + getCodec().writeValue(this, rootNode); + } + } + + @Override + public void writeString(String text) throws IOException { + super.writeString(escape(text)); + } + + @Override + public void writeString(char[] text, int offset, int len) throws IOException { + super.writeString(escape(new String(text, offset, len))); + } + + @Override + public void writeString(SerializableString text) throws IOException { + super.writeString(escape(text.getValue())); + } + + private static String escape(String text) { + return text!=null && !text.isEmpty() && FORMULA_TRIGGER_CHARS.indexOf(text.charAt(0))>=0 + ? "'"+text : text; + } } }