Skip to content

Potential fix for code scanning alert no. 6: Unsigned difference expression compared to zero - #478

Merged
mrjimenez merged 1 commit into
masterfrom
alert-autofix-6
Apr 25, 2026
Merged

Potential fix for code scanning alert no. 6: Unsigned difference expression compared to zero#478
mrjimenez merged 1 commit into
masterfrom
alert-autofix-6

Conversation

@mrjimenez

Copy link
Copy Markdown
Contributor

Potential fix for https://github.com/amule-project/amule/security/code-scanning/6

Use a direct relational comparison that does not perform unsigned subtraction in the condition.

Best fix in this snippet:

  • In src/EncryptedStreamSocket.cpp, inside CEncryptedStreamSocket::SendNegotiatingData, replace:
    • if (nBufLen - nStartCryptFromByte > 0) {
  • With:
    • if (nBufLen > nStartCryptFromByte) {

This preserves behavior when the invariant holds, avoids underflow-prone expression patterns, satisfies CodeQL, and does not change functionality. No new imports, helper methods, or type changes are needed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ession compared to zero

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@mrjimenez
mrjimenez marked this pull request as ready for review April 25, 2026 15:27
@mrjimenez
mrjimenez merged commit ace1a6f into master Apr 25, 2026
13 checks passed
@mrjimenez
mrjimenez deleted the alert-autofix-6 branch April 25, 2026 15:37
got3nks added a commit to got3nks/amule that referenced this pull request Jul 13, 2026
…oject#478)

aMuleGUI's lists (Downloads, Shared, Searches, servers/Kad) are the generic
custom-drawn wxGenericListCtrl, not the native ListView. On Windows the view
leaves rows unpainted when it scrolls without a full invalidation: entire lines
go blank on mouse/scrollbar scroll and on HOME/END/PgUp/PgDn, reappearing only
on select, CTRL+A, or a scroll back. The mouse/scrollbar case regressed
recently (clean on 3.0.1 stable per the reporter); the keyboard case is a
longer-standing wxMSW quirk.

Enable double-buffering on the CMuleListCtrl base (WS_EX_COMPOSITED on wxMSW),
which paints the control and its inner list window through an off-screen
buffer, so every scroll repaints the whole viewport. Scoped to __WXMSW__ --
macOS/GTK already double-buffer. This fixes the paint without touching the
Freeze()/Thaw() bulk-load batching (amule-project#414), which stays for its perf win.

Refs amule-project#478
got3nks added a commit to got3nks/amule that referenced this pull request Jul 13, 2026
…t#478)

amule-project#348 replaced the vendored wxListMainWindow::OnScroll's synchronous
HandleOnScroll(event) with event.Skip() (HandleOnScroll became private in wx
3.3.3). On wxMSW the deferred scroll path (wxScrollHelperBase::HandleOnScroll
-> ScrollWindow) blits the retained rows and only invalidates the newly
exposed strip, which is then left unpainted -- rows go blank on mouse/scrollbar
scroll until a redraw is forced (select, CTRL+A, scroll back). Verified in the
wx 3.2.10 and 3.3.x sources that the scroll/repaint mechanism is identical, so
this is a wxMSW platform behavior, not a wx-version one: event.Skip() blanks on
Windows regardless of wx version.

Force a full Refresh() after the scroll on __WXMSW__ only. macOS/GTK repaint the
exposed region correctly on their own and are untouched; the repaint is
flicker-free because wxListMainWindow::OnPaint uses a wxBufferedPaintDC. Scoped
by platform rather than wx version so it stays correct if the Windows build
ever moves to wx 3.3.x, and it avoids the now-private HandleOnScroll entirely.

Does not address the pre-existing keyboard (HOME/END/PgUp/PgDn) blanking, which
is a separate, older path.

Refs amule-project#478
got3nks added a commit to got3nks/amule that referenced this pull request Jul 14, 2026
…mule-project#478)

amule-project#348 replaced the vendored wxListMainWindow::OnScroll's synchronous
HandleOnScroll(event) with event.Skip() -- HandleOnScroll became a private
member in wx 3.3.3, so the macOS/Linux CI (on wx 3.3.x) stopped compiling. On
wxMSW the resulting deferred-scroll path (wxScrollHelperBase::HandleOnScroll ->
ScrollWindow) blits the retained rows and only invalidates the newly exposed
strip, which is then left unpainted: rows go blank on mouse-wheel, scrollbar and
pagination scrolling until a redraw is forced (select, CTRL+A, scroll back). It
also forced a smoother/laggier scroll feel. Verified in the wx 3.2.10 and 3.3.x
sources that the scroll+repaint mechanism is identical, so this is a wxMSW
platform behavior, not a wx-version one.

Rather than gate on platform or wx version (both fragile -- the former would need
the now-private HandleOnScroll, the latter breaks once Windows ships wx 3.3.3),
reimplement the synchronous scroll inline using only the public wxScrollHelper
API (GetViewStart / GetScrollLines / GetScrollPageSize / Scroll), mirroring
HandleOnScroll()/CalcScrollInc(): translate the scroll event into a target
position in scroll units and Scroll() to it (which clamps and does the actual
ScrollWindow + repaint). A pre-scroll Update() flushes pending repaints so the
blit starts from valid content, exactly as HandleOnScroll did. Not skipping the
event means the base scroll helper won't also scroll, so no double-scroll.

Works on every supported wx (>= 3.2.0) and every platform with no version or
platform guard; compiles against wx 3.3.3. Does not address the pre-existing
keyboard (HOME/END/PgUp/PgDn) blanking, which is a separate, older path.

Refs amule-project#478
got3nks added a commit to got3nks/amule that referenced this pull request Jul 14, 2026
amule-project#478)

amule-project#348 replaced the vendored wxListMainWindow::OnScroll's synchronous
HandleOnScroll(event) with event.Skip() -- HandleOnScroll became a private
member in wx 3.3.3, so the macOS/Linux CI (on wx 3.3.x) stopped compiling. On
wxMSW the resulting deferred-scroll path (wxScrollHelperBase::HandleOnScroll ->
ScrollWindow) blits the retained rows and only invalidates the newly exposed
strip, which is then left unpainted: rows go blank on mouse-wheel, scrollbar and
pagination scrolling until a redraw is forced. Verified in the wx 3.2.10 and
3.3.x sources that the scroll+repaint mechanism is identical, so this is a wxMSW
platform behavior, not a wx-version one.

Rather than gate on platform or wx version (both fragile -- the former needs the
now-private HandleOnScroll, the latter breaks once Windows ships wx 3.3.3),
reimplement the synchronous scroll using only the public wxScrollHelper API
(GetViewStart / GetScrollLines / GetScrollPageSize / Scroll), mirroring
HandleOnScroll()/CalcScrollInc(): translate the scroll event into a target
position in scroll units and scroll to it.

Factor the "scroll + repaint" sequence into a shared ScrollListTo(x, y) helper
(Update() to flush pending paints so the blit is clean, Scroll(), then
ResetVisibleLinesRange() so the exposed rows repaint) and route both OnScroll
(scrollbar/wheel) and MoveToItem (keyboard nav) through it. This also fixes the
keyboard HOME/END/PgUp/PgDn blanking, which was the same bug from a different
path: MoveToItem only reset the visible-line range after Scroll() under
__WXMAC__, so on Windows the range stayed stale and keyboard scrolls came up
blank. The reset now runs on every platform via the shared helper, and the old
__WXMAC__-only workaround is gone.

Not skipping the event means the base scroll helper won't also scroll, so no
double-scroll. Works on every supported wx (>= 3.2.0) and every platform with no
version or platform guard; compiles against wx 3.3.3.

Refs amule-project#478
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Jul 15, 2026
…ject#445) + list rows blanking on scroll/keyboard (amule-project#478) (amule-project#477)

* fix(remote-gui): stop the log view blanking on Windows by dropping Freeze/Thaw (amule-project#445)

amule-project#471 tried to fix the "log view goes blank on every new line" Windows
regression by thawing before scrolling, but it made no difference: the
AppendText still ran while the control was frozen. Appending to a frozen
wxTE_RICH2 (RichEdit) on Windows leaves its line/scroll metrics stale, so on
Thaw the view renders blank with the newest line pinned to the top until a
manual scroll forces a recompute. The Freeze()/Thaw() that amule-project#451 wrapped the
per-poll appends in is the actual culprit, not the scroll order.

Drop the Freeze()/Thaw() and append on a live control (as the pre-amule-project#451 code
did). Keep the two real wins from amule-project#451: the daemon's 5000-lines-per-poll cap
and the conditional SetDefaultStyle, plus one coalesced ShowPosition per poll
instead of per line (m_logBatching still suppresses the per-line scroll). The
per-line SetDefaultStyle was the dominant first-sync cost, so responsiveness is
retained without the frozen-append rendering corruption.

macOS/GTK recompute metrics regardless and were unaffected either way.

* fix(remote-gui): repaint list rows on scroll & keyboard nav on Windows (amule-project#478)

amule-project#348 replaced the vendored wxListMainWindow::OnScroll's synchronous
HandleOnScroll(event) with event.Skip() -- HandleOnScroll became a private
member in wx 3.3.3, so the macOS/Linux CI (on wx 3.3.x) stopped compiling. On
wxMSW the resulting deferred-scroll path (wxScrollHelperBase::HandleOnScroll ->
ScrollWindow) blits the retained rows and only invalidates the newly exposed
strip, which is then left unpainted: rows go blank on mouse-wheel, scrollbar and
pagination scrolling until a redraw is forced. Verified in the wx 3.2.10 and
3.3.x sources that the scroll+repaint mechanism is identical, so this is a wxMSW
platform behavior, not a wx-version one.

Rather than gate on platform or wx version (both fragile -- the former needs the
now-private HandleOnScroll, the latter breaks once Windows ships wx 3.3.3),
reimplement the synchronous scroll using only the public wxScrollHelper API
(GetViewStart / GetScrollLines / GetScrollPageSize / Scroll), mirroring
HandleOnScroll()/CalcScrollInc(): translate the scroll event into a target
position in scroll units and scroll to it.

Factor the "scroll + repaint" sequence into a shared ScrollListTo(x, y) helper
(Update() to flush pending paints so the blit is clean, Scroll(), then
ResetVisibleLinesRange() so the exposed rows repaint) and route both OnScroll
(scrollbar/wheel) and MoveToItem (keyboard nav) through it. This also fixes the
keyboard HOME/END/PgUp/PgDn blanking, which was the same bug from a different
path: MoveToItem only reset the visible-line range after Scroll() under
__WXMAC__, so on Windows the range stayed stale and keyboard scrolls came up
blank. The reset now runs on every platform via the shared helper, and the old
__WXMAC__-only workaround is gone.

Not skipping the event means the base scroll helper won't also scroll, so no
double-scroll. Works on every supported wx (>= 3.2.0) and every platform with no
version or platform guard; compiles against wx 3.3.3.

Refs amule-project#478
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant