From 53c432a635d3c195bd111c2ba6643905b2503ff0 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Sat, 2 May 2026 23:25:41 +0200 Subject: [PATCH] fix(security): close codeql findings #1 and #2 Two CodeQL alerts opened against the codeql-manual-build workflow's first scan. Both real, both small fixes. #1 Medium / Workflow does not contain permissions build.yml runs read-only against the repo (no push, no release creation, no API mutations) but never declared a permissions block, so the default GITHUB_TOKEN scope applied. Pin to contents: read at workflow level. Release and CodeQL workflows already have their explicit minimal scopes. #2 Critical / Unvalidated local pointer arithmetic ImGuiUtil.WrappedTextWithPos splits its input on newlines and passes each part through Encoding.UTF8.GetBytes inside a fixed block. Empty splits (consecutive newlines, blank lines) produced a zero-length byte array, fixed gave us a valid pointer, and textEnd = text + bytes.Length collapsed onto text. The downstream ImGuiNative.CalcWordWrapPositionA calls received identical start and end pointers, which is undefined behaviour at the native boundary even if it happens to no-op on the current ImGui build. Bail before entering the fixed block when bytes.Length == 0 and render an empty line for the gap, which is what the original text == null guard was trying to do but could never reach inside a fixed block over a non-null array. --- .github/workflows/build.yml | 7 +++++++ ChatTwo/Util/ImGuiUtil.cs | 20 +++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 196e7a8..af16057 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,6 +11,13 @@ on: branches: [main] workflow_dispatch: +# Minimum permissions for a build-only workflow: read the repo, nothing +# else. Closes the CodeQL "Workflow does not contain permissions" alert +# and matches the principle-of-least-privilege the security guide +# recommends for workflows that don't push or create releases. +permissions: + contents: read + jobs: build: name: Build (Release) diff --git a/ChatTwo/Util/ImGuiUtil.cs b/ChatTwo/Util/ImGuiUtil.cs index 7379001..97deb15 100755 --- a/ChatTwo/Util/ImGuiUtil.cs +++ b/ChatTwo/Util/ImGuiUtil.cs @@ -94,18 +94,24 @@ internal static class ImGuiUtil foreach (var part in csText.Split(["\r\n", "\r", "\n"], StringSplitOptions.None)) { var bytes = Encoding.UTF8.GetBytes(part); + + // Empty splits (consecutive newlines) leave bytes.Length at 0 + // and the textEnd pointer below would coincide with text. The + // ImGuiNative word-wrap calls treat that as undefined input, + // and the CodeQL "unvalidated local pointer arithmetic" alert + // also flags it. Render an empty line and skip the unsafe + // block entirely for this iteration. + if (bytes.Length == 0) + { + ImGui.TextUnformatted(""); + continue; + } + fixed (byte* rawText = bytes) { var text = rawText; var textEnd = text + bytes.Length; - // empty string - if (text == null) - { - ImGui.TextUnformatted(""); - continue; - } - var widthLeft = ImGui.GetContentRegionAvail().X; var endPrevLine = ImGuiNative.CalcWordWrapPositionA(ImGui.GetFont().Handle, ImGuiHelpers.GlobalScale, text, textEnd, widthLeft); if (endPrevLine == null)