nightshift: security-footgun — 18 findings (3 critical anti-forensics)

[nightshift-micr]

nightshift: Security Footgun Scanner — Microck/veyoff

Summary

Security analysis of veyoff's 2,301-line C++ Windows MITM proxy (src/windows/veyoff-windows.cpp). Found 18 security-relevant findings across 5 severity levels. The tool intercepts RFB/VNC traffic between Veyon's classroom monitoring and UltraVNC — inherently operating in a security-sensitive domain.

---

Critical (Severity 1)

1. Event log clearing in self-destruct destroys forensic evidence

File: veyoff-windows.cpp:1484-1498
Function: clearEventLogs()

Clears Application, System, and Security event logs. Clearing the Security log is a serious anti-forensics action that:

• Removes evidence of ALL security events on the system (not just veyoff-related)
• Requires audit privileges (SE_AUDIT_NAME) — the admin check elsewhere doesn't verify this specific privilege
• May trigger a SIEM/alert if configured
• Violates most enterprise security policies even on personal machines

The README describes this as a feature ("clears windows event logs"), but the Security log clearing goes well beyond cleaning up veyoff's own traces.

2. Self-destruct schedules cmd.exe to delete arbitrary directory

File: veyoff-windows.cpp:1500+ (self-destruct continuation)

The self-destruct mechanism spawns cmd.exe with ping delay + rd /s /q on the executable's directory. If the executable is in a shared or system directory (e.g., %TEMP% that other processes use, or a directory with other files), this destroys unrelated data. There's no validation that the target directory only contains veyoff files.

3. Prefetch clearing with wildcard matching

File: veyoff-windows.cpp:1500+

clearPrefetch() deletes files from C:\Windows\Prefetch matching the executable name. This is forensic anti-evidence behavior that could mask other malicious processes. The function uses RemoveDirectory + DeleteFile patterns that could fail silently or succeed on wrong files if the naming is broad.

---

High (Severity 2)

4. Registry modification without backup

File: veyoff-windows.cpp:475-485
Function: writeVncPortToRegistry(int port)

Writes directly to HKLM\SOFTWARE\Veyon Solutions\Veyon\Network\VncServerPort without backing up the original value first. If veyoff crashes between write and the service restart, the Veyon configuration is left in a modified state with no automatic recovery. The only recovery is the clean-quit path (Ctrl+Alt+Q).

5. Service control with no privilege escalation check

File: veyoff-windows.cpp:487-506
Function: controlService()

Opens SC_MANAGER_CONNECT and manipulates VeyonService without first checking if the current user has the required privileges. If run as a standard user, OpenSCManagerW fails silently and returns false, but the proxy continues to bind the port and intercept traffic without properly redirecting Veyon. This creates an inconsistent state where Veyon can't connect but veyoff is still running.

6. Socket operations with no TLS/encryption

File: veyoff-windows.cpp:219-241

All RFB traffic between the proxy and UltraVNC is plaintext. While RFB/VNC itself doesn't typically use TLS (Veyon adds its own auth layer), the proxy sits on localhost so this is mitigated. However, if someone modifies the code to listen on non-loopback, all screen data is transmitted unencrypted.

7. nameLen in ServerInit parsed without upper bound

File: veyoff-windows.cpp:1005-1009

uint32_t nameLen = readU32(serverInit + 20);
if (nameLen > 0 && nameLen < 65536) {
    if (!forwardBytes(session.serverSock, session.clientSock, nameLen)) return false;
}

The 64KB limit prevents the worst case, but a malicious VNC server could still send a 64KB name string that gets forwarded. Not exploitable in the intended use case (UltraVNC on localhost), but a defense-in-depth concern.

8. Blacklist loaded from file with no integrity check

File: veyoff-windows.cpp:250-262
Function: loadBlacklist()

The blacklist file is read with no validation of:

• File permissions (any user/process can modify it)
• File size limits (a very large file causes excessive matching)
• Content encoding (non-UTF-8 bytes are silently converted)
• Symlink following (a symlink could redirect to arbitrary files)

A local attacker could modify the blacklist to hide their own windows from the teacher's view, or add so many entries that the matching loop becomes a DoS.

---

Medium (Severity 3)

9. Global mutable state with coarse locking

File: veyoff-windows.cpp:587-611
Struct: SharedState

A single std::mutex protects all shared state including frozenFrame (potentially megabytes of pixel data). Copying the frozen frame while holding the lock blocks the GUI thread, the proxy threads, and the hotkey handler. This could cause visible stuttering during freeze/unfreeze operations.

10. FrameBuffer copies are deep copies of std::vector<uint8_t>

File: veyoff-windows.cpp:364-371

The FrameBuffer struct contains a std::vector<uint8_t> that's copied by value in multiple places (e.g., getInterceptFrame copies teacherVisibleFrame while holding the mutex). For a 1920×1080 screen at 32bpp, each copy is ~8MB. This happens on every frame update when blacklist is active (500ms poll interval = ~16MB/s allocation rate).

11. sendAll/recvAll have no timeout

File: veyoff-windows.cpp:219-241

These functions loop until all bytes are sent/received, with no timeout. If one end of the socket becomes unresponsive (e.g., VNC server hangs), the forwarding thread blocks indefinitely. Combined with the coarse mutex, this could deadlock the entire proxy.

12. No input validation on port parameter from registry

File: veyoff-windows.cpp:462-473
Function: readVncPortFromRegistry()

Reads a DWORD from the registry and casts it to int. If the registry value is 0, negative, or > 65535, it's used directly as a port number. htons(static_cast<u_short>(port)) with an out-of-range port silently wraps around, potentially binding on an unexpected port.

13. Settings file path derived from blacklist path without validation

File: veyoff-windows.cpp:264-268
Function: deriveSettingsPath()

settings.ini is written to the same directory as the blacklist. If the blacklist is in a system directory or a directory with strict ACLs, WritePrivateProfileStringW fails silently. No error is surfaced to the user — overlay settings appear to save but are lost.

---

Low (Severity 4)

14. SetEncodings rewrite strips all encodings except Raw

File: veyoff-windows.cpp:1138-1160

The proxy rewrites the client's SetEncodings to only request Raw encoding from the server. This forces uncompressed transmission for ALL frames, even when not intercepting. In normal (non-frozen, non-blacklist) mode, this unnecessarily increases bandwidth between UltraVNC and the proxy.

15. buildRawFrameUpdate doesn't handle zero-dimension framebuffers

File: veyoff-windows.cpp:666-739

Early return if sendW <= 0 || sendH <= 0, which is correct. But fbWidth/fbHeight of 0 from a malformed ServerInit would cause the proxy to silently skip all frame updates, making the teacher see a blank screen without any error message.

16. Window enumeration for blacklist could be resource-intensive

File: veyoff-windows.cpp:336-360
Function: enumWindowsProc

Called for every visible window on the system, performing string matching against the full blacklist for each. With many windows and a long blacklist, this runs on every frame capture (500ms interval). The string matching is O(n*m) where n=blacklist entries, m=windows.

17. No cleanup of orphaned proxy sessions

File: veyoff-windows.cpp:782-793
Function: activeSessionCount()

Dead sessions are pruned in activeSessionCount(), but this function is only called when checking session count. If sessions die without anyone calling activeSessionCount(), they accumulate in the vector. Over a long-running session with connection drops, memory slowly leaks.

---

Informational (Severity 5)

18. Overlay window uses WDA_EXCLUDEFROMCAPTURE — Windows 10 2004+ only

File: veyoff-windows.cpp:51-53

The WDA_EXCLUDEFROMCAPTURE constant (0x11) is defined as a fallback for older SDK headers. On Windows 10 versions before 2004, this API doesn't exist and SetWindowDisplayAffinity fails silently. The overlay window may be visible to screen capture on older systems.

---

Recommendations

1. Remove Security log clearing from clearEventLogs() — clearing Application and System is sufficient for trace removal and is less destructive.
2. Validate the self-destruct target directory — ensure it only contains veyoff files before deleting.
3. Add file integrity checks for the blacklist (checksum, size limit, permission check).
4. Add port range validation after reading from registry (1-65535, non-well-known).
5. Use shared_ptr or move semantics for FrameBuffer to avoid deep copies under the mutex.
6. Add timeouts to sendAll/recvAll using select() or WSAPoll().
7. Document the security implications of the self-destruct feature prominently in the README.