From f0f37df60217c34924cc5c32fb1d2af7db703617 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 10 Sep 2026 11:06:44 -0500 Subject: [PATCH 1/2] wip-security updates --- .agents/skills/release-notes/SKILL.md | 198 + .codex/config.toml | 7 + WebContent/WEB-INF/jsp/assessment/list.jsp | 3 +- WebContent/WEB-INF/jsp/client/Access.jsp | 5 +- WebContent/WEB-INF/jsp/client/Client.jsp | 5 +- .../WEB-INF/jsp/cms/ReportingTemplates.jsp | 2 +- WebContent/WEB-INF/jsp/cms/TemplateUpload.jsp | 4 +- WebContent/WEB-INF/jsp/cms/templates.jsp | 2 +- WebContent/WEB-INF/jsp/header.jsp | 7 +- .../WEB-INF/jsp/peerreviews/TrackChanges.jsp | 14 +- WebContent/WEB-INF/jsp/register/newuser.jsp | 4 +- WebContent/WEB-INF/jsp/register/reset.jsp | 4 +- WebContent/bootstrap/css/bootstrap.css | 1084 +- WebContent/bootstrap/css/bootstrap.css.map | 2 +- WebContent/bootstrap/css/bootstrap.min.css | 7 +- WebContent/bootstrap/js/bootstrap.js | 619 +- WebContent/bootstrap/js/bootstrap.min.js | 9 +- WebContent/dist/js/cms.js | 2 +- WebContent/dist/js/footer.js | 2 +- WebContent/dist/js/install_extension.js | 2 +- WebContent/index.jsp | 4 +- WebContent/package-lock.json | 15 + WebContent/package.json | 1 + .../datatables/dataTables.bootstrap.css | 699 +- .../datatables/dataTables.bootstrap.js | 148 +- .../datatables/dataTables.bootstrap.min.js | 12 +- .../plugins/datatables/jquery.dataTables.js | 13168 ++++++++-------- .../datatables/jquery.dataTables.min.js | 162 +- WebContent/plugins/jQuery/jQuery-2.1.4.min.js | 4 - WebContent/plugins/jQuery/jquery-3.7.1.min.js | 2 + .../jQuery/jquery-migrate-3.6.0.min.js | 3 + WebContent/src/appstore/install_extension.js | 6 +- WebContent/src/cms/cms.js | 4 +- WebContent/src/footer.js | 2 +- WebContent/src/others.js | 7 +- mise.toml | 2 +- src/com/fuse/actions/admin/Options.java | 25 +- .../actions/appstore/AppStoreController.java | 2 + .../appstore/InstallExtensionController.java | 45 +- src/com/fuse/dao/AppStore.java | 19 +- src/com/fuse/extenderapi/Extensions.java | 34 +- src/com/fuse/utils/FSUtils.java | 184 +- test/com/fuse/unittests/CryptoTests.java | 105 + 43 files changed, 9025 insertions(+), 7610 deletions(-) create mode 100644 .agents/skills/release-notes/SKILL.md create mode 100644 .codex/config.toml delete mode 100644 WebContent/plugins/jQuery/jQuery-2.1.4.min.js create mode 100644 WebContent/plugins/jQuery/jquery-3.7.1.min.js create mode 100644 WebContent/plugins/jQuery/jquery-migrate-3.6.0.min.js create mode 100644 test/com/fuse/unittests/CryptoTests.java diff --git a/.agents/skills/release-notes/SKILL.md b/.agents/skills/release-notes/SKILL.md new file mode 100644 index 00000000..090599dd --- /dev/null +++ b/.agents/skills/release-notes/SKILL.md @@ -0,0 +1,198 @@ +--- +name: release-notes +description: Draft release notes for the next FACTION release by reading commit diffs since the previous tag and pushing them to GitHub as a DRAFT release via `gh release create --draft` (or updating an existing draft). Use when the user asks to draft/prepare release notes, summarize what's changed since a tag, or prepare a release writeup. Never publishes — always draft. +--- + +# FACTION release notes + +You are drafting human-quality release notes for the FACTION project. The goal +is the kind of writeup a user reads in a GitHub Release and immediately +understands what changed, why it matters to them, and whether they need to +do anything to upgrade. + +Keyword-matching commit subjects produces shallow output. **Read the diffs.** +That is the difference between "Add default-vulnerability CRUD endpoints" (a +commit subject) and a release-notes paragraph that lists each new endpoint, +its method/path, and the round-trip-cleanly fix for names containing `/`. + +## Step 1 — Pick the version + +If the user passed a version (e.g. `/release-notes 1.8.6`), use it. + +Otherwise derive it from `pom.xml`: + +```bash +grep -m1 '' pom.xml | sed -E 's/.*([^<]+)<\/version>.*/\1/' | sed 's/-SNAPSHOT//' +``` + +If the result still contains `SNAPSHOT` or is empty, stop and ask the user +which version this release should be tagged as. + +## Step 2 — Pick the previous tag + +Default to the most recent annotated/lightweight tag: + +```bash +git describe --tags --abbrev=0 +``` + +If that errors (no tags exist), ask the user for a starting ref instead of +guessing. If the user passed a second arg, use that as the previous tag. + +## Step 3 — Inspect what changed + +Run these (in parallel where independent): + +```bash +git log --reverse ..HEAD --pretty=format:'%h %s' +git diff --stat ..HEAD +``` + +Then **for each non-trivial commit**, read the actual diff: + +```bash +git show --stat +git show -- +``` + +Skip commits whose subject starts with `[maven-release-plugin]` — they are +version-bump noise. Also skip pure-internal hygiene (gitignore tweaks, +formatting-only changes) unless that's all that landed. + +If `.github/release.yml` exists, read it — its `categories` section names +match the GitHub Release auto-categorizer (currently +`🎉 🚀 Upgrades 🎉 🚀` and `🐛 Bugfixes 🐛`). Use those exact section +titles so the file mirrors what GitHub would generate. + +## Step 4 — Draft on GitHub + +You will push a draft release to GitHub rather than committing a markdown +file to the repo. The user reviews and publishes from +. + +Preflight: + +```bash +gh auth status # verify gh is installed and logged in +gh release view # check if a release for this version already exists +``` + +- If `gh auth status` fails, stop and tell the user to run `gh auth login`. +- If `gh release view` succeeds **and** the release is published (not a + draft), stop and ask before overwriting — published releases should not be + silently mutated. +- If `gh release view` succeeds and the release **is** a draft, you'll + update it in place via `gh release edit` (see below). +- If `gh release view` fails with "release not found", you'll create a new + draft via `gh release create`. + +Compose the notes body using the structure below, then write it to a temp +file (`/tmp/release-notes-.md`) — passing markdown via `--notes` +inline is fragile with backticks and code fences, so always use +`--notes-file`. + +Body structure (omit any section that's empty): + +The body starts directly with the executive summary — do **not** add a +`# FACTION ` title or a `_Release date: ..._` line. GitHub already +renders the release title and date from the release object itself. + +```markdown + + +## 🎉 🚀 Upgrades 🎉 🚀 + +### + + + +## 🐛 Bugfixes 🐛 + +- + +## 🧰 Internal / Test infrastructure + + + +## Upgrade notes + +- **Database migration:** required / not required (state which). +- **API:** call out any breaking changes, or explicitly say "all existing + endpoints continue to work unchanged" if true. +- **Configuration:** any new required env vars or settings. +- **Permissions:** any new permission scopes or role changes. + +## Full changelog + +...> +``` + +## Style rules + +- **Read diffs, don't paraphrase commit subjects.** A commit titled "fix bug" + tells the reader nothing; the diff tells you what actually changed. +- **User-facing voice.** "You can now …" beats "We added a method that …". +- **Group by feature, not by commit.** Three commits that together + implement one endpoint become one bullet. +- **Tables for API additions.** Method / path / purpose. Always. +- **Backward compatibility is load-bearing.** If old clients keep working, + say so explicitly — that's often the most reassuring sentence in the file. +- **No emojis inside body text.** They're fine in the section headers + (because `.github/release.yml` uses them), but don't sprinkle them through + the prose. +- **No "Co-Authored-By" anywhere.** This project's commits and release + notes never carry that trailer. + +## Step 5 — Push the draft + +Write the composed body to the temp file, then create or update the draft. + +**New draft** (no existing release for ``): + +```bash +gh release create \ + --draft \ + --title "FACTION " \ + --notes-file /tmp/release-notes-.md \ + --target main +``` + +`--draft` is mandatory — never publish from this skill. The tag does not +have to exist yet; GitHub creates it at `--target` only when the draft is +published. + +**Existing draft** (re-running the skill to iterate on the same release): + +```bash +gh release edit \ + --draft \ + --notes-file /tmp/release-notes-.md +``` + +`gh release edit` keeps the existing title unless `--title` is also passed. +Only pass `--title` if the user explicitly asked to rename the release. + +## Hand-off + +After the `gh` command succeeds: +1. Print the draft URL. `gh release create` prints it on stdout; for the + edit path, get it with `gh release view --json url -q .url`. +2. Print the first ~10 lines of the body so the user can sanity-check the + framing without leaving the terminal. +3. Tell the user the draft is **unpublished** and that they should review + on GitHub before clicking Publish. +4. Do **not** publish, `git tag`, or `git push` anything. The skill's + contract ends at "draft is up for review." + +## When gh isn't available + +If `gh` is not installed or the user is offline, fall back to writing +`RELEASE_NOTES_.md` at the repo root and tell the user explicitly +that you did so because GitHub was unreachable — they can paste it into +the Releases UI manually. diff --git a/.codex/config.toml b/.codex/config.toml new file mode 100644 index 00000000..e1e9b122 --- /dev/null +++ b/.codex/config.toml @@ -0,0 +1,7 @@ +[mcp_servers.faction] +command = "node" +args = ["/Volumes/extended/Code/faction-all/free/faction-mcp/dist/index.js"] + +[mcp_servers.faction.env] +FACTION_API_KEY = "bf9bff1a-fc5d-4e86-adc3-86bb9165d180" +FACTION_BASE_URL = "https://acme.factionsecurity.com" diff --git a/WebContent/WEB-INF/jsp/assessment/list.jsp b/WebContent/WEB-INF/jsp/assessment/list.jsp index 4ff45c37..4e3d4597 100644 --- a/WebContent/WEB-INF/jsp/assessment/list.jsp +++ b/WebContent/WEB-INF/jsp/assessment/list.jsp @@ -181,7 +181,8 @@ color:#00a65a + + --> diff --git a/WebContent/WEB-INF/jsp/client/Access.jsp b/WebContent/WEB-INF/jsp/client/Access.jsp index 8b9a0da1..585511b5 100644 --- a/WebContent/WEB-INF/jsp/client/Access.jsp +++ b/WebContent/WEB-INF/jsp/client/Access.jsp @@ -88,8 +88,9 @@ - - + + + diff --git a/WebContent/WEB-INF/jsp/client/Client.jsp b/WebContent/WEB-INF/jsp/client/Client.jsp index 570fab43..92af84a4 100644 --- a/WebContent/WEB-INF/jsp/client/Client.jsp +++ b/WebContent/WEB-INF/jsp/client/Client.jsp @@ -98,8 +98,9 @@ - - + + + diff --git a/WebContent/WEB-INF/jsp/cms/ReportingTemplates.jsp b/WebContent/WEB-INF/jsp/cms/ReportingTemplates.jsp index 2aa6a1af..4caae386 100644 --- a/WebContent/WEB-INF/jsp/cms/ReportingTemplates.jsp +++ b/WebContent/WEB-INF/jsp/cms/ReportingTemplates.jsp @@ -92,7 +92,7 @@ - + diff --git a/WebContent/WEB-INF/jsp/cms/TemplateUpload.jsp b/WebContent/WEB-INF/jsp/cms/TemplateUpload.jsp index 9b1921a0..293a6347 100644 --- a/WebContent/WEB-INF/jsp/cms/TemplateUpload.jsp +++ b/WebContent/WEB-INF/jsp/cms/TemplateUpload.jsp @@ -36,10 +36,10 @@ - + - + diff --git a/WebContent/WEB-INF/jsp/cms/templates.jsp b/WebContent/WEB-INF/jsp/cms/templates.jsp index dcb31ede..0b03c207 100644 --- a/WebContent/WEB-INF/jsp/cms/templates.jsp +++ b/WebContent/WEB-INF/jsp/cms/templates.jsp @@ -35,7 +35,7 @@ height: 700px; < - ${name }${team.teamName} + diff --git a/WebContent/WEB-INF/jsp/header.jsp b/WebContent/WEB-INF/jsp/header.jsp index 1a159d60..14b9a996 100644 --- a/WebContent/WEB-INF/jsp/header.jsp +++ b/WebContent/WEB-INF/jsp/header.jsp @@ -25,8 +25,9 @@ - - + + + @@ -57,7 +58,7 @@   ${_title1} ${_title2} + src="../tri-logo.png" />   diff --git a/WebContent/WEB-INF/jsp/peerreviews/TrackChanges.jsp b/WebContent/WEB-INF/jsp/peerreviews/TrackChanges.jsp index 50cda270..701fc003 100644 --- a/WebContent/WEB-INF/jsp/peerreviews/TrackChanges.jsp +++ b/WebContent/WEB-INF/jsp/peerreviews/TrackChanges.jsp @@ -133,7 +133,7 @@ span.Informational {

- Peer Review for [${asmt.appId}] - ${asmt.name } - ${asmt.assessor[0].fname} ${asmt.assessor[0].lname} + Peer Review for [] - -

@@ -168,7 +168,7 @@ span.Informational {
- +
Notes
@@ -189,7 +189,7 @@ span.Informational {
- +
Notes
@@ -260,7 +260,7 @@ span.Informational {
Description
- +
Description Notes
@@ -276,7 +276,7 @@ span.Informational {
Recommendation
- +
Recommendation Notes
@@ -292,7 +292,7 @@ span.Informational {
Details
- +
Detail Notes
@@ -375,7 +375,7 @@ span.Informational { - + Incomplete diff --git a/WebContent/WEB-INF/jsp/register/newuser.jsp b/WebContent/WEB-INF/jsp/register/newuser.jsp index 736806ff..bceeb7e1 100644 --- a/WebContent/WEB-INF/jsp/register/newuser.jsp +++ b/WebContent/WEB-INF/jsp/register/newuser.jsp @@ -71,7 +71,9 @@ User Name and/or Password is invalid - + + + + + + @@ -262,7 +263,7 @@ content: content }); - }).error(function(){ + }).fail(function(){ $.alert({ title: 'Error!', content: "There is a problem with your request." diff --git a/mise.toml b/mise.toml index 3b3c8438..e50399d2 100644 --- a/mise.toml +++ b/mise.toml @@ -92,7 +92,7 @@ run = "docker-compose -f docker-compose-hotreload.yml restart tomcat-service" [tasks."hotreload:logs"] description = "HotReload: View Logs" -run = "docker-compose -f docker-compose-hotreload.yml logs -f tomcat-service" +run = "docker compose -f docker-compose-hotreload.yml logs -f tomcat-service" [tasks."hotreload:stop"] description = "HotReload: Stop" diff --git a/src/com/fuse/actions/admin/Options.java b/src/com/fuse/actions/admin/Options.java index 4dd13311..96a8a072 100644 --- a/src/com/fuse/actions/admin/Options.java +++ b/src/com/fuse/actions/admin/Options.java @@ -129,6 +129,10 @@ public String execute() { this._message = "Name is Empty"; return this.ERRORJSON; } + if (FSUtils.containsHTML(this.name)) { + this._message = "Assessment Type name cannot contain HTML"; + return this.ERRORJSON; + } AssessmentType AT = AssessmentQueries.getAssessmentTypeByName(em, this.name); if (AT != null) { @@ -544,12 +548,21 @@ public String updateTiltes() { if (!this.testToken(false)) return this.ERRORJSON; + if (this.title == null || this.title.length < 2) { + this._message = "Both titles are required"; + return this.ERRORJSON; + } + if (FSUtils.containsHTML(this.title[0]) || FSUtils.containsHTML(this.title[1])) { + this._message = "Titles cannot contain HTML"; + return this.ERRORJSON; + } + EMS = (SystemSettings) em.createQuery("from SystemSettings").getResultList().stream().findFirst().orElse(null); if (EMS == null) { EMS = new SystemSettings(); } - EMS.setBoldTitle(this.title[0]); - EMS.setOtherTitle(this.title[1]); + EMS.setBoldTitle(this.title[0].trim()); + EMS.setOtherTitle(this.title[1].trim()); HibHelper.getInstance().preJoin(); em.joinTransaction(); em.persist(EMS); @@ -634,6 +647,14 @@ public String editType() { this._message = "Assessment Type does not exist"; return this.ERRORJSON; } + if (this.getName() == null || this.getName().trim().equals("")) { + this._message = "Name is Empty"; + return this.ERRORJSON; + } + if (FSUtils.containsHTML(this.getName())) { + this._message = "Assessment Type name cannot contain HTML"; + return this.ERRORJSON; + } AssessmentType t2 = AssessmentQueries.getAssessmentTypeByName(em, this.getName()); diff --git a/src/com/fuse/actions/appstore/AppStoreController.java b/src/com/fuse/actions/appstore/AppStoreController.java index 114137e6..254a8a3d 100644 --- a/src/com/fuse/actions/appstore/AppStoreController.java +++ b/src/com/fuse/actions/appstore/AppStoreController.java @@ -80,6 +80,7 @@ public String enableApp() { HibHelper.getInstance().preJoin(); em.joinTransaction(); em.persist(app); + AuditLog.audit(this, "Extension enabled: " + app.getName() + " " + app.getVersion(), AuditLog.UserAction, false); HibHelper.getInstance().commit(); } _result="success"; @@ -95,6 +96,7 @@ public String disableApp() { HibHelper.getInstance().preJoin(); em.joinTransaction(); em.persist(app); + AuditLog.audit(this, "Extension disabled: " + app.getName() + " " + app.getVersion(), AuditLog.UserAction, false); HibHelper.getInstance().commit(); _result="success"; return MESSAGEJSON; diff --git a/src/com/fuse/actions/appstore/InstallExtensionController.java b/src/com/fuse/actions/appstore/InstallExtensionController.java index b110da03..eee7876b 100644 --- a/src/com/fuse/actions/appstore/InstallExtensionController.java +++ b/src/com/fuse/actions/appstore/InstallExtensionController.java @@ -73,9 +73,15 @@ public String uploadUpdate() throws IOException, ParseException { .stream() .findFirst() .orElse(null); - - FileInputStream fis = new FileInputStream(file_data); - app.updateApp(fis); + if (app == null || file_data == null) { + return this.errorJson("Extension not found or no file uploaded"); + } + try (FileInputStream fis = new FileInputStream(file_data)) { + app.updateApp(fis); + } catch (Exception ex) { + AuditLog.audit(this, "Rejected extension update upload: " + ex.getMessage(), AuditLog.UserAction, true); + return this.errorJson("Invalid extension JAR: " + ex.getMessage()); + } String json = app.getMeta(); ServletActionContext.getRequest().getSession().setAttribute("PreviewApp", app); stream = new ByteArrayInputStream(json.toString().getBytes()); @@ -89,19 +95,37 @@ public String uploadUpdate() throws IOException, ParseException { @Result(name = "input", location = "/WEB-INF/jsp/uploadError.jsp") }) public String uploadFile() throws IOException, ParseException { - FileInputStream fis = new FileInputStream(file_data); + if (file_data == null) { + return this.errorJson("No file uploaded"); + } AppStore preview = new AppStore(); - preview.parseJar(fis); + try (FileInputStream fis = new FileInputStream(file_data)) { + preview.parseJar(fis); + } catch (Exception ex) { + AuditLog.audit(this, "Rejected extension upload: " + ex.getMessage(), AuditLog.UserAction, true); + return this.errorJson("Invalid extension JAR: " + ex.getMessage()); + } String json = preview.getMeta(); ServletActionContext.getRequest().getSession().setAttribute("PreviewApp", preview); stream = new ByteArrayInputStream(json.toString().getBytes()); return "json"; } + + private String errorJson(String message) { + String json = "{\"error\": \"" + message.replace("\\", "\\\\").replace("\"", "\\\"") + "\"}"; + stream = new ByteArrayInputStream(json.getBytes()); + return "json"; + } @Action(value = "InstallApp") public String installApp() throws IOException, ParseException { AppStore app = (AppStore) ServletActionContext.getRequest().getSession().getAttribute("PreviewApp"); + if (app == null) { + _message="No extension has been uploaded for review"; + _result="error"; + return MESSAGEJSON; + } Boolean alreadyInstalled =em.createQuery("from AppStore where hash = :hash") .setParameter("hash", app.getHash()) .getResultList() @@ -116,7 +140,10 @@ public String installApp() throws IOException, ParseException { HibHelper.getInstance().preJoin(); em.joinTransaction(); em.persist(app); + AuditLog.audit(this, "Extension installed: " + app.getName() + " " + app.getVersion() + " (hash " + app.getHash() + ")", + AuditLog.UserAction, false); HibHelper.getInstance().commit(); + ServletActionContext.getRequest().getSession().removeAttribute("PreviewApp"); _result="success"; return MESSAGEJSON; @@ -125,6 +152,11 @@ public String installApp() throws IOException, ParseException { @Action(value = "UpdateApp") public String updateApp() throws IOException, ParseException { AppStore app = (AppStore) ServletActionContext.getRequest().getSession().getAttribute("PreviewApp"); + if (app == null) { + _message="No extension has been uploaded for review"; + _result="error"; + return MESSAGEJSON; + } List apps =em.createQuery("from AppStore where hash = :hash or uuid = :uuid") .setParameter("hash", app.getHash()) .setParameter("uuid", app.getUuid()) @@ -156,7 +188,10 @@ public String updateApp() throws IOException, ParseException { HibHelper.getInstance().preJoin(); em.joinTransaction(); em.merge(app); + AuditLog.audit(this, "Extension updated: " + app.getName() + " " + app.getVersion() + " (hash " + app.getHash() + ")", + AuditLog.UserAction, false); HibHelper.getInstance().commit(); + ServletActionContext.getRequest().getSession().removeAttribute("PreviewApp"); _result="success"; return MESSAGEJSON; diff --git a/src/com/fuse/dao/AppStore.java b/src/com/fuse/dao/AppStore.java index b340fcf1..75d18a72 100644 --- a/src/com/fuse/dao/AppStore.java +++ b/src/com/fuse/dao/AppStore.java @@ -296,6 +296,9 @@ public void parseJar(FileInputStream fis) throws IOException, ParseException { JarInputStream jarStream = new JarInputStream(fis); Manifest manifest = jarStream.getManifest(); + if (manifest == null) { + throw new IOException("Extension JAR is missing META-INF/MANIFEST.MF"); + } Attributes attr = manifest.getMainAttributes(); String title = attr.getValue("Title"); String author = attr.getValue("Author"); @@ -338,22 +341,28 @@ public void parseJar(FileInputStream fis) throws IOException, ParseException { logo.write(data, 0, size); } } - if (!entry.isDirectory() && entry.getName().endsWith("com.faction.extender.ApplicationInventory")) { + if (!entry.isDirectory() && entry.getName().equals("META-INF/services/com.faction.extender.ApplicationInventory")) { isInventoryApp = true; } - if (!entry.isDirectory() && entry.getName().endsWith("com.faction.extender.AssessmentManager")) { + if (!entry.isDirectory() && entry.getName().equals("META-INF/services/com.faction.extender.AssessmentManager")) { isAssessmentApp = true; } - if (!entry.isDirectory() && entry.getName().endsWith("com.faction.extender.ReportManager")) { + if (!entry.isDirectory() && entry.getName().equals("META-INF/services/com.faction.extender.ReportManager")) { isReportingApp=true; } - if (!entry.isDirectory() && entry.getName().endsWith("com.faction.extender.VulnerabilityManager")) { + if (!entry.isDirectory() && entry.getName().equals("META-INF/services/com.faction.extender.VulnerabilityManager")) { isVulnerabilityApp = true; } - if (!entry.isDirectory() && entry.getName().endsWith("com.faction.extender.VerificationManager")) { + if (!entry.isDirectory() && entry.getName().equals("META-INF/services/com.faction.extender.VerificationManager")) { isVerificationApp = true; } } + if (!(isAssessmentApp || isVerificationApp || isVulnerabilityApp || isInventoryApp || isReportingApp)) { + throw new IOException("Extension JAR does not register any FACTION extension service (META-INF/services/com.faction.extender.*)"); + } + if (title == null || author == null || version == null || url == null) { + throw new IOException("Extension JAR manifest must define Title, Author, Version and URL"); + } //reset file pointer FileChannel fc = fis.getChannel(); fc.position(0); diff --git a/src/com/fuse/extenderapi/Extensions.java b/src/com/fuse/extenderapi/Extensions.java index b821afb1..1c542bb7 100644 --- a/src/com/fuse/extenderapi/Extensions.java +++ b/src/com/fuse/extenderapi/Extensions.java @@ -575,8 +575,14 @@ public void loadExtensions() throws MalformedURLException { List apps = this.sortApps(); for (AppStore app : apps) { - + // sortApps() only returns extensions that are enabled *and* registered for this + // event type, so bytecode from a disabled (or unrelated) extension is never + // loaded into the JVM. ServiceLoader runs static initialisers and constructors + // as soon as it iterates, so the filtering has to happen before this point. URLClassLoader extensionLoader = dynamicExtensionClassLoader(app); + if (extensionLoader == null) { + continue; + } ClassLoader currentClassLoader = Thread.currentThread().getContextClassLoader(); // Load Assessment Manager Extensions @@ -667,16 +673,40 @@ public void loadExtensions() throws MalformedURLException { } } + /** + * Installed extensions that are enabled and have opted in to the event type this + * instance serves, in configured order. Disabled extensions are never returned and + * therefore never class-loaded. + */ private List sortApps() { EntityManager em = entityManagerFactory.createEntityManager(); try { List apps = em.createQuery("from AppStore order by order").getResultList(); - return apps; + return apps.stream() + .filter(app -> Boolean.TRUE.equals(app.getEnabled()) && this.isEnabledFor(app)) + .collect(Collectors.toList()); } finally { em.close(); } } + private boolean isEnabledFor(AppStore app) { + switch (this.type) { + case ASMT_MANAGER: + return Boolean.TRUE.equals(app.getAssessmentEnabled()); + case REPORT_MANAGER: + return Boolean.TRUE.equals(app.getReportEnabled()); + case VULN_MANAGER: + return Boolean.TRUE.equals(app.getVulnerabilityEnabled()); + case VER_MANAGER: + return Boolean.TRUE.equals(app.getVerificationEnabled()); + case INVENTORY: + return Boolean.TRUE.equals(app.getInventoryEnabled()); + default: + return false; + } + } + public List getLogs() { return this.logs; diff --git a/src/com/fuse/utils/FSUtils.java b/src/com/fuse/utils/FSUtils.java index 675dd1de..8fb92477 100644 --- a/src/com/fuse/utils/FSUtils.java +++ b/src/com/fuse/utils/FSUtils.java @@ -15,6 +15,7 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; +import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.spec.KeySpec; @@ -43,6 +44,7 @@ import javax.crypto.Cipher; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; import javax.persistence.EntityManager; @@ -187,6 +189,15 @@ public static String sanitizeHTML(String html) { } + /** + * True when a value contains characters that could open an HTML tag. Used to + * reject plain-text settings (titles, type names) that are later shown in the + * UI. Output encoding is still applied at render time; this is defence in depth. + */ + public static boolean containsHTML(String value) { + return value != null && (value.indexOf('<') >= 0 || value.indexOf('>') >= 0); + } + public static String sanitizeGUID(String guid) { String regexGUID = "^[0-9a-zA-Z\\-].*$"; Matcher guids = Pattern.compile(regexGUID).matcher(guid); @@ -456,54 +467,101 @@ private static void createCategories(EntityManager em) { } + // --------------------------------------------------------------------- + // Symmetric encryption of stored secrets (SMTP/LDAP passwords, API keys, + // keystores, report passwords). The key is derived from FACTION_SECRET_KEY. + // + // Current format ("v2"): $AESGCM$ + base64( salt[16] || iv[12] || AES-GCM ciphertext+tag ) + // - a fresh random salt and IV are generated for every value, so identical + // plaintexts never produce identical ciphertexts and the PBKDF2 salt is + // never reused. + // Legacy format (pre 1.8.14): base64( AES/ECB ciphertext ) with a constant + // PBKDF2 salt. It is still *read* so that values already in the database + // keep working; it is never written. Values are re-encrypted in the new + // format the next time they are saved. + // --------------------------------------------------------------------- + private static final String GCM_PREFIX = "$AESGCM$"; + private static final int GCM_SALT_LENGTH = 16; + private static final int GCM_IV_LENGTH = 12; + private static final int GCM_TAG_BITS = 128; + private static final int PBKDF2_ITERATIONS = 65536; + private static final int AES_KEY_BITS = 256; + private static final byte[] LEGACY_SALT = "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(); + + private static SecretKey deriveKey(byte[] salt) throws GeneralSecurityException { + String secret = getEnv("FACTION_SECRET_KEY"); + if (secret == null || secret.isEmpty()) { + throw new IllegalStateException("FACTION_SECRET_KEY is not set"); + } + MessageDigest md = MessageDigest.getInstance("SHA-256"); + byte[] hash = md.digest(secret.getBytes(StandardCharsets.UTF_8)); + char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); + + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); + KeySpec spec = new PBEKeySpec(b64hash, salt, PBKDF2_ITERATIONS, AES_KEY_BITS); + SecretKey tmp = factory.generateSecret(spec); + return new SecretKeySpec(tmp.getEncoded(), "AES"); + } + + private static String encryptToString(byte[] plaintext) throws GeneralSecurityException { + SecureRandom random = new SecureRandom(); + byte[] salt = new byte[GCM_SALT_LENGTH]; + byte[] iv = new byte[GCM_IV_LENGTH]; + random.nextBytes(salt); + random.nextBytes(iv); + + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + cipher.init(Cipher.ENCRYPT_MODE, deriveKey(salt), new GCMParameterSpec(GCM_TAG_BITS, iv)); + byte[] ciphertext = cipher.doFinal(plaintext); + + byte[] out = new byte[GCM_SALT_LENGTH + GCM_IV_LENGTH + ciphertext.length]; + System.arraycopy(salt, 0, out, 0, GCM_SALT_LENGTH); + System.arraycopy(iv, 0, out, GCM_SALT_LENGTH, GCM_IV_LENGTH); + System.arraycopy(ciphertext, 0, out, GCM_SALT_LENGTH + GCM_IV_LENGTH, ciphertext.length); + return GCM_PREFIX + Base64.encodeBase64String(out); + } + + private static byte[] decryptToBytes(String data) throws GeneralSecurityException { + if (data == null) { + throw new IllegalArgumentException("nothing to decrypt"); + } + if (data.startsWith(GCM_PREFIX)) { + byte[] raw = Base64.decodeBase64(data.substring(GCM_PREFIX.length())); + int headerLength = GCM_SALT_LENGTH + GCM_IV_LENGTH; + if (raw.length < headerLength + (GCM_TAG_BITS / 8)) { + throw new GeneralSecurityException("ciphertext too short"); + } + byte[] salt = Arrays.copyOfRange(raw, 0, GCM_SALT_LENGTH); + byte[] iv = Arrays.copyOfRange(raw, GCM_SALT_LENGTH, headerLength); + byte[] ciphertext = Arrays.copyOfRange(raw, headerLength, raw.length); + + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + cipher.init(Cipher.DECRYPT_MODE, deriveKey(salt), new GCMParameterSpec(GCM_TAG_BITS, iv)); + return cipher.doFinal(ciphertext); + } + // Legacy AES/ECB value written by an earlier release. Read-only. + Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding"); + cipher.init(Cipher.DECRYPT_MODE, deriveKey(LEGACY_SALT)); + return cipher.doFinal(Base64.decodeBase64(data)); + } + public static String decryptPassword(String password) { try { - MessageDigest md = MessageDigest.getInstance("SHA-256"); - String secret = System.getenv("FACTION_SECRET_KEY"); - byte[] hash = md.digest(secret.getBytes()); - char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); - - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - KeySpec spec = new PBEKeySpec(b64hash, "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(), 65536, 256); - SecretKey tmp = factory.generateSecret(spec); - SecretKey SecKey = new SecretKeySpec(tmp.getEncoded(), "AES"); - - Cipher AesCipher = Cipher.getInstance("AES"); - AesCipher.init(Cipher.DECRYPT_MODE, SecKey); - byte[] cypherText = Base64.decodeBase64(password); - byte[] bytePlainText = AesCipher.doFinal(cypherText); - return new String(bytePlainText); - + return new String(decryptToBytes(password), StandardCharsets.UTF_8); } catch (Exception ex) { return ""; } - } - public static byte [] decryptBytes(String data) { - try { - MessageDigest md = MessageDigest.getInstance("SHA-256"); - String secret = System.getenv("FACTION_SECRET_KEY"); - byte[] hash = md.digest(secret.getBytes()); - char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); - - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - KeySpec spec = new PBEKeySpec(b64hash, "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(), 65536, 256); - SecretKey tmp = factory.generateSecret(spec); - SecretKey SecKey = new SecretKeySpec(tmp.getEncoded(), "AES"); - - Cipher AesCipher = Cipher.getInstance("AES"); - AesCipher.init(Cipher.DECRYPT_MODE, SecKey); - byte[] cypherText = Base64.decodeBase64(data); - byte[] bytePlainText = AesCipher.doFinal(cypherText); - return bytePlainText; + public static byte[] decryptBytes(String data) { + try { + return decryptToBytes(data); } catch (Exception ex) { System.out.println(ex); return null; } - } - + public static String md5hash(String data) { try { MessageDigest md; @@ -529,58 +587,20 @@ public static String md5hash(byte [] data) { public static String encryptPassword(String password) { try { - - MessageDigest md = MessageDigest.getInstance("SHA-256"); - String secret = System.getenv("FACTION_SECRET_KEY"); - byte[] hash = md.digest(secret.getBytes()); - char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); - - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - KeySpec spec = new PBEKeySpec(b64hash, "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(), 65536, 256); - SecretKey tmp = factory.generateSecret(spec); - SecretKey SecKey = new SecretKeySpec(tmp.getEncoded(), "AES"); - - Cipher AesCipher = Cipher.getInstance("AES"); - - byte[] byteText = password.getBytes(); - - AesCipher.init(Cipher.ENCRYPT_MODE, SecKey); - byte[] byteCipherText = AesCipher.doFinal(byteText); - - return Base64.encodeBase64String(byteCipherText); - - } catch (Exception Ex) { - Ex.printStackTrace(); + return encryptToString(password.getBytes(StandardCharsets.UTF_8)); + } catch (Exception ex) { + ex.printStackTrace(); return null; } - } - public static String encryptBytes(byte [] data) { - try { - MessageDigest md = MessageDigest.getInstance("SHA-256"); - String secret = System.getenv("FACTION_SECRET_KEY"); - byte[] hash = md.digest(secret.getBytes()); - char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); - - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - KeySpec spec = new PBEKeySpec(b64hash, "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(), 65536, 256); - SecretKey tmp = factory.generateSecret(spec); - SecretKey SecKey = new SecretKeySpec(tmp.getEncoded(), "AES"); - - Cipher AesCipher = Cipher.getInstance("AES"); - - - AesCipher.init(Cipher.ENCRYPT_MODE, SecKey); - byte[] byteCipherText = AesCipher.doFinal(data); - - return Base64.encodeBase64String(byteCipherText); - - } catch (Exception Ex) { - Ex.printStackTrace(); + public static String encryptBytes(byte[] data) { + try { + return encryptToString(data); + } catch (Exception ex) { + ex.printStackTrace(); return null; } - } diff --git a/test/com/fuse/unittests/CryptoTests.java b/test/com/fuse/unittests/CryptoTests.java new file mode 100644 index 00000000..d1c3c94d --- /dev/null +++ b/test/com/fuse/unittests/CryptoTests.java @@ -0,0 +1,105 @@ +package com.fuse.unittests; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.spec.KeySpec; + +import javax.crypto.Cipher; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; + +import org.apache.commons.codec.binary.Base64; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.fuse.utils.FSUtils; + +/** + * Covers the at-rest encryption helpers in FSUtils: the AES-GCM format written + * today and the read-only fallback for values written by earlier releases in + * AES/ECB with a constant salt. + */ +public class CryptoTests { + + private static final String SECRET = "unit-test-secret-key"; + + @BeforeClass + public static void setSecret() { + if (System.getenv("FACTION_SECRET_KEY") == null) { + System.setProperty("FACTION_SECRET_KEY", SECRET); + } + } + + private static String secret() { + String env = System.getenv("FACTION_SECRET_KEY"); + return env == null ? SECRET : env; + } + + @Test + public void passwordRoundTrip() { + String encrypted = FSUtils.encryptPassword("p@ssw0rd with spaces and ünïcode"); + assertTrue("new values must use the GCM format", encrypted.startsWith("$AESGCM$")); + assertEquals("p@ssw0rd with spaces and ünïcode", FSUtils.decryptPassword(encrypted)); + } + + @Test + public void bytesRoundTrip() { + byte[] data = new byte[1024]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) i; + } + String encrypted = FSUtils.encryptBytes(data); + assertTrue(encrypted.startsWith("$AESGCM$")); + assertArrayEquals(data, FSUtils.decryptBytes(encrypted)); + } + + @Test + public void sameInputProducesDifferentCiphertext() { + String first = FSUtils.encryptPassword("same"); + String second = FSUtils.encryptPassword("same"); + assertNotEquals("random salt/IV must make ciphertexts unique", first, second); + assertEquals("same", FSUtils.decryptPassword(first)); + assertEquals("same", FSUtils.decryptPassword(second)); + } + + @Test + public void tamperedCiphertextIsRejected() { + String encrypted = FSUtils.encryptPassword("integrity"); + byte[] raw = Base64.decodeBase64(encrypted.substring("$AESGCM$".length())); + raw[raw.length - 1] ^= 0x01; // flip a bit in the GCM tag + String tampered = "$AESGCM$" + Base64.encodeBase64String(raw); + assertEquals("", FSUtils.decryptPassword(tampered)); + } + + @Test + public void legacyEcbValuesStillDecrypt() throws Exception { + // Reproduce exactly what encryptPassword() wrote before the GCM format existed. + MessageDigest md = MessageDigest.getInstance("SHA-256"); + byte[] hash = md.digest(secret().getBytes()); + char[] b64hash = Base64.encodeBase64String(hash).toCharArray(); + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); + KeySpec spec = new PBEKeySpec(b64hash, "f04ce910-bedb-4d8f-a023-4d2441dc0fba".getBytes(), 65536, 256); + SecretKey tmp = factory.generateSecret(spec); + SecretKey key = new SecretKeySpec(tmp.getEncoded(), "AES"); + Cipher legacy = Cipher.getInstance("AES/ECB/PKCS5Padding"); + legacy.init(Cipher.ENCRYPT_MODE, key); + String legacyValue = Base64.encodeBase64String(legacy.doFinal("old-smtp-password".getBytes(StandardCharsets.UTF_8))); + + assertEquals("old-smtp-password", FSUtils.decryptPassword(legacyValue)); + } + + @Test + public void garbageInputIsHandled() { + assertEquals("", FSUtils.decryptPassword(null)); + assertEquals("", FSUtils.decryptPassword("")); + assertEquals("", FSUtils.decryptPassword("$AESGCM$AAAA")); + assertEquals("", FSUtils.decryptPassword("not-base64-!!")); + } +} From a6968cb4d26b1c33476cc9fdf46350aebee77ce0 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 10 Sep 2026 14:58:58 -0500 Subject: [PATCH 2/2] Sanitize peer review notes to close stored XSS The peer review view (TrackChanges, prqueue=true) renders each field into a suneditor rich-text editor, which reads the textarea value and injects it as innerHTML. Output-encoding the textarea does not help there, so any stored HTML in the "notes" fields executed in a reviewer's browser. Sanitize desc_notes/rec_notes/detail_notes on Vulnerability and summary1_notes/summary2_notes on Comment, matching the existing description/recommendation/details handling. Comment.exportAssessment rebuilds each vulnerability through these setters on every display, so this also neutralizes notes stored before the fix. Track-changes markup is preserved. Adds NotesXssSanitizationTest and bumps the version to 1.8.14-SNAPSHOT. --- pom.xml | 2 +- src/com/fuse/dao/Comment.java | 6 +- src/com/fuse/dao/Vulnerability.java | 6 +- .../unittests/NotesXssSanitizationTest.java | 67 +++++++++++++++++++ 4 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 test/com/fuse/unittests/NotesXssSanitizationTest.java diff --git a/pom.xml b/pom.xml index b2a25fa5..e9db1046 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.faction faction - 1.8.13-SNAPSHOT + 1.8.14-SNAPSHOT war Faction diff --git a/src/com/fuse/dao/Comment.java b/src/com/fuse/dao/Comment.java index e45edb4c..5ca59459 100644 --- a/src/com/fuse/dao/Comment.java +++ b/src/com/fuse/dao/Comment.java @@ -1,5 +1,7 @@ package com.fuse.dao; +import com.fuse.utils.FSUtils; + import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -128,11 +130,11 @@ public void setSummary2(String summary2) { } public void setSummary1_notes(String summary1_notes) { - this.summary1_notes = summary1_notes; + this.summary1_notes = FSUtils.sanitizeHTML(summary1_notes); } public void setSummary2_notes(String summary2_notes) { - this.summary2_notes = summary2_notes; + this.summary2_notes = FSUtils.sanitizeHTML(summary2_notes); } public List getCommenters() { diff --git a/src/com/fuse/dao/Vulnerability.java b/src/com/fuse/dao/Vulnerability.java index abb0449f..1f13bd93 100644 --- a/src/com/fuse/dao/Vulnerability.java +++ b/src/com/fuse/dao/Vulnerability.java @@ -274,7 +274,7 @@ public String getDesc_notes() { } @Transient public void setDesc_notes(String desc_notes) { - this.desc_notes = desc_notes; + this.desc_notes = FSUtils.sanitizeHTML(desc_notes); } @Transient public String getRec_notes() { @@ -282,7 +282,7 @@ public String getRec_notes() { } @Transient public void setRec_notes(String rec_notes) { - this.rec_notes = rec_notes; + this.rec_notes = FSUtils.sanitizeHTML(rec_notes); } public String getCvssScore() { return cvssScore == null? "" : cvssScore; @@ -329,7 +329,7 @@ public String getDetail_notes() { } @Transient public void setDetail_notes(String detail_notes) { - this.detail_notes = detail_notes; + this.detail_notes = FSUtils.sanitizeHTML(detail_notes); } public Boolean getDesc_lock() { return desc_lock; diff --git a/test/com/fuse/unittests/NotesXssSanitizationTest.java b/test/com/fuse/unittests/NotesXssSanitizationTest.java new file mode 100644 index 00000000..244cf5a6 --- /dev/null +++ b/test/com/fuse/unittests/NotesXssSanitizationTest.java @@ -0,0 +1,67 @@ +package com.fuse.unittests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.fuse.dao.Comment; +import com.fuse.dao.Vulnerability; + +/** + * Peer review "notes" fields are rendered into live suneditor rich-text editors + * on the TrackChanges page (peerreviewedit.js). Output-encoding the textarea does + * not help there, because the editor reads textarea.value (already entity-decoded) + * and injects it as innerHTML. The only defense is sanitizing the stored value, so + * these setters must strip active content the way the description/recommendation/ + * details setters already do. Comment.exportAssessment() reconstructs a vuln through + * these same setters on every display, so this also neutralizes notes that were + * stored before the fix. + */ +public class NotesXssSanitizationTest { + + private static final String IMG = ""; + private static final String SCRIPT = ""; + private static final String TRACKED = "ok added"; + + private static void assertStripped(String value) { + assertFalse("onerror handler must be removed: " + value, value.toLowerCase().contains("onerror")); + assertFalse("script tag must be removed: " + value, value.toLowerCase().contains("

"); + assertEquals("

", v.getDesc_notes()); + } +}