Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/osv-scanner.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@ id = "GHSA-84m7-p3x7-pcfv"
ignoreUntil = 2026-11-16
reason = "mcp 1.29.0 is here only because semgrep requires it, and CI runs semgrep as semgrep scan. The advisory is about an MCP server over Streamable HTTP, and nothing in this repository serves MCP. The fix is mcp 1.30.0, but every semgrep release from 1.180.0 to 1.182.1 requires mcp==1.29.0 exactly (read on the package index on 2026-10-26), so pip cannot install it. Remove this entry once a semgrep release allows mcp 1.30.0 or later."

[[IgnoredVulns]]
id = "GHSA-5h93-6whr-6q8j"
ignoreUntil = 2026-11-16
reason = "mcp 1.29.0 is here only because semgrep requires it, for the reason given at GHSA-84m7-p3x7-pcfv above. The advisory, published 2026-10-27, is about the MCP client's HTTP transports following cross-origin redirects with custom headers and bodies. CI runs semgrep as semgrep scan (.github/workflows/security.yml), which acts as no MCP client and connects to no MCP server. The fix is mcp 1.30.0, and every semgrep release up to 1.182.1, the newest on 2026-10-27, requires mcp==1.29.0 exactly. Remove this entry once a semgrep release allows mcp 1.30.0 or later."

[[IgnoredVulns]]
id = "GHSA-rwrf-2pqf-9j8j"
ignoreUntil = 2026-11-16
reason = "mcp 1.29.0 is here only because semgrep requires it, for the reason given at GHSA-84m7-p3x7-pcfv above. The advisory, published 2026-10-27, is about the MCP client fetching $ref URLs chosen by a server while it validates tool results. CI runs semgrep as semgrep scan, which acts as no MCP client and validates no tool result. The fix is mcp 1.30.0, which pip cannot install next to any semgrep release up to 1.182.1. Remove this entry once a semgrep release allows mcp 1.30.0 or later."

[[IgnoredVulns]]
id = "GHSA-w4fh-qvv9-3v23"
ignoreUntil = 2026-11-16
reason = "mcp 1.29.0 is here only because semgrep requires it, for the reason given at GHSA-84m7-p3x7-pcfv above. The advisory, published 2026-10-27, is about an MCP server accepting bearer tokens issued for a different resource server. Nothing in this repository serves MCP, and CI runs semgrep as semgrep scan, which starts no server. The fix is mcp 1.30.0, which pip cannot install next to any semgrep release up to 1.182.1. Remove this entry once a semgrep release allows mcp 1.30.0 or later."

[[IgnoredVulns]]
id = "GHSA-xcq7-p3pf-p88h"
ignoreUntil = 2026-11-16
Expand Down
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ is not part of this repository.
read".
- The window cannot be made narrower than 1000 points, up from 900, so the new button fits in the
bar of actions.
- A snapshot now keeps what exactly each trigger waits for: its subtype - the device class, the
event provider, or which of a kind's two halves it is, such as a port opening rather than closing -
and its data, such as the RPC interface, the hardware ID or the port and protocol. Until now it kept
only the kind and the action, so two machines whose triggers waited for different things compared as
the same. Snapshots are now schema version 7, and this version still reads versions 4 to 6. A
comparison of a version 7 snapshot with an older one compares triggers by kind and action only and
says so once. Between two version 7 snapshots the data is compared exactly, including upper and lower
case, because Windows matches some of it exactly. The same subtype and data are in `bws list --json`
and `bws show --json`, as `subtype` and a `data` list of items with a `type` and their `values`. In
the JSON of a comparison the new caveat is `triggerDetailDiffers`, at the end of `caveats`. An older
version of this tool refuses a version 7 snapshot by name.

### Fixed

Expand Down
10 changes: 8 additions & 2 deletions src/Bws.Cli/DiffJson.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ internal static string Render(SnapshotDiff diff) =>
diff.Caveats.LanguageDiffers,
diff.Caveats.AccountDiffers,
diff.Caveats.NotKnown,
diff.Caveats.FileVersionSourceDiffers),
diff.Caveats.FileVersionSourceDiffers,
diff.Caveats.TriggerDetailDiffers),
[.. diff.Added.Select(Presence)],
[.. diff.Removed.Select(Presence)],
[.. diff.Uncertain.Select(Presence)],
Expand Down Expand Up @@ -128,6 +129,10 @@ private sealed record RefusedDocument(string Field, IReadOnlyList<string> Entrie
/// document only grew at its end. True when one side is schema six and the other older: file
/// versions were then not compared on any entry.
/// </param>
/// <param name="TriggerDetailDiffers">
/// Added 2026-10-27 (logic report C01, schema seven) and LAST, for the same reason. True when one side
/// is schema seven and the other older: triggers were then compared by kind and action only.
/// </param>
private sealed record CaveatsDocument(
bool ElevationDiffers,
bool MachineDiffers,
Expand All @@ -136,7 +141,8 @@ private sealed record CaveatsDocument(
bool LanguageDiffers,
bool AccountDiffers,
IReadOnlyList<string> NotKnown,
bool FileVersionSourceDiffers);
bool FileVersionSourceDiffers,
bool TriggerDetailDiffers);

private sealed record LeftOutDocument(int Earlier, int Later);

Expand Down
1 change: 1 addition & 0 deletions src/Bws.Cli/DiffText.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ private static void Caveats(StringBuilder text, SnapshotDiff diff)
(caveats.LanguageDiffers, () => Texts.Of("cli.diff.caveat.language")),
(caveats.AccountDiffers, () => Texts.Of("cli.diff.caveat.account")),
(caveats.FileVersionSourceDiffers, () => Texts.Of("cli.diff.caveat.fileVersion")),
(caveats.TriggerDetailDiffers, () => Texts.Of("cli.diff.caveat.triggers")),
(caveats.NotKnown.Count > 0, () => Texts.Of("cli.diff.caveat.notKnown", string.Join(", ", caveats.NotKnown))),
(diff.NeitherRead.Count > 0, () => Texts.Of("cli.diff.caveat.neitherRead", string.Join(", ", diff.NeitherRead))),
(diff.LeftOut.Any, () => Texts.Of("cli.diff.caveat.instancesLeftOut", diff.LeftOut.Earlier, diff.LeftOut.Later))
Expand Down
1 change: 1 addition & 0 deletions src/Bws.Cli/Resources/cli.en.json
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@
"cli.diff.caveat.language": "The service manager names things in a different language on each side, so display names and descriptions were not compared.",
"cli.diff.caveat.account": "The two snapshots were taken by different accounts. What one account may read, the other may be refused.",
"cli.diff.caveat.fileVersion": "One snapshot was written before this tool read each file's version from the file itself - older versions could take it from a language file beside it - so file versions were not compared.",
"cli.diff.caveat.triggers": "One snapshot was written before this tool kept what exactly each trigger waits for, so triggers were compared by kind and action only.",
"cli.diff.caveat.notKnown": "At least one of the snapshots does not record these - it was written before they were, or the machine would not say - so they could not be checked: {0}",
"cli.diff.caveat.instancesLeftOut": "Per-user session copies come and go with the people signed in, so they are not compared - earlier snapshot: {0}, later snapshot: {1}.",
"cli.diff.caveat.neitherRead": "On at least one entry neither snapshot holds these fields, so those entries were not compared on them: {0}",
Expand Down
248 changes: 248 additions & 0 deletions src/Bws.Core/ManagerBlocks.Triggers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
using System.Buffers.Binary;
using System.Globalization;
using System.Runtime.InteropServices;
using Windows.Win32.System.Services;

namespace Bws.Core;

/// <summary>
/// The trigger walk: the records of a trigger block, and since 2026-10-27 everything each record points
/// at - its subtype and its data items (logic report C01, snapshot schema seven).
///
/// <b>Its own file since that day</b>, because the walk grew from one array to three levels of pointers
/// into one block, and it is one subject with one way of going wrong: every level is a count or a pointer
/// the manager wrote, and none of them is believed until it is checked against the block.
/// </summary>
internal static partial class ManagerBlocks
{
/// <summary>
/// The triggers inside a block the manager has already filled.
///
/// <b>Its own method so that a test can hand it a block, which is the only way anything below
/// can be checked at all.</b> The half above needs a service handle and therefore a machine
/// with a service on it - this half needs bytes, and the bytes that matter are the ones a
/// machine does not produce on demand. Same shape as <see cref="ReadDependentNames"/> and
/// <see cref="ReadConfigurationBuffer"/>, for the same reason. <b>That sentence named those
/// readers as "left in WindowsScmCatalog" until 2026-09-02, and the last of them arrived in this
/// class that day</b> - the seam was named in this header long before it was taken, and what
/// finally took it was the size ratchet on the file they were leaving.
///
/// <b>BOUNDED BY THE BLOCK SINCE 2026-08-26, and until then this walked a count and a pointer
/// that both came from the manager, with neither checked against the block they describe.</b>
/// Every other buffer here limits itself - <see cref="ReadMultiString"/> by the pointer landing
/// inside, the enumeration and configuration buffers by their own length. This was the one
/// left, and it is the one where being wrong costs the most: a record past the end is an access
/// violation, which ends the process outright with no report, possibly in the middle of listing
/// eight hundred entries on somebody else's server. Owner's decision of 2026-08-03, applied in
/// the last place that had not had it.
///
/// <b>The pointer is checked as well as the count</b>, because either can be wrong on its own.
/// It closes a second way this could have gone wrong as well: the structure points into the
/// very block the call filled, so an array that moved between that call and the fixed statement
/// below would leave an offset nowhere near the buffer - refused here rather than followed.
///
/// <b>A block that does not describe itself comes back as nothing read rather than as no
/// triggers.</b> An empty list would be a claim about the service - that it declares none -
/// and that claim is exactly what could not be established.
///
/// <b>What a record points at is refused rather than held down, unlike the record count</b>
/// (2026-10-27). A count of records larger than the block keeps the records that fit, as it has
/// since 2026-08-26. A subtype or a data item outside the block refuses the whole field with
/// ERROR_INVALID_DATA, the way a configuration field outside its block is refused since R-4 -
/// because a trigger with one of its data items quietly dropped is a different trigger: a device
/// trigger fires on any of its hardware IDs and an ETW trigger on its filters, so one item fewer
/// widens or narrows what the entry waits for, and the reading would say so as fact.
/// </summary>
internal static unsafe Reading<IReadOnlyList<ServiceTrigger>> ReadTriggerBuffer(byte[] buffer)
{
ArgumentNullException.ThrowIfNull(buffer);

if (buffer.Length < sizeof(SERVICE_TRIGGER_INFO))
{
return Reading<IReadOnlyList<ServiceTrigger>>.Absent();
}

fixed (byte* start = buffer)
{
var info = *(SERVICE_TRIGGER_INFO*)start;

if (info.cTriggers == 0)
{
// A fact about the service: most entries have none.
return Reading<IReadOnlyList<ServiceTrigger>>.Absent();
}

var offset = (byte*)info.pTriggers - start;

if (info.pTriggers is null || offset < 0 || offset >= buffer.Length)
{
return Reading<IReadOnlyList<ServiceTrigger>>.Absent();
}

var room = (uint)((buffer.Length - offset) / sizeof(SERVICE_TRIGGER));
var count = Math.Min(info.cTriggers, room);

if (count == 0)
{
return Reading<IReadOnlyList<ServiceTrigger>>.Absent();
}

var triggers = new List<ServiceTrigger>((int)count);

// READ THROUGH THE OFFSET THAT WAS CHECKED, NOT THROUGH THE POINTER THAT WAS NOT, and
// until 2026-09-02 this loop did the second. The check above establishes that the
// offset lands inside the block, and then the reading went back to info.pTriggers -
// so in the one case the check exists for, a pointer landing in range by accident, it
// validated one address and dereferenced another. A guard that looks like a guard and
// is not is worse than no guard, because the next reader stops looking. Backlog 297.
var records = (SERVICE_TRIGGER*)(start + offset);

return Records(records, count, start, buffer.Length, triggers)
? Reading<IReadOnlyList<ServiceTrigger>>.Present(triggers)
: OutsideTheBlock<IReadOnlyList<ServiceTrigger>>();
}
}

/// <summary>Every record whole into <paramref name="triggers"/>, or false at the first that is not.</summary>
private static unsafe bool Records(
SERVICE_TRIGGER* records, uint count, byte* block, int length, List<ServiceTrigger> triggers)
{
for (uint index = 0; index < count; index++)
{
if (Whole(records[index], block, length) is not { } trigger)
{
return false;
}

triggers.Add(trigger);
}

return true;
}

/// <summary>
/// One record with its subtype and its data items, or null when anything it points at is outside
/// the block or is not the size its type says.
/// </summary>
private static unsafe ServiceTrigger? Whole(SERVICE_TRIGGER record, byte* block, int length)
{
Guid? subtype = null;

if (record.pTriggerSubtype is not null)
{
if (Within(record.pTriggerSubtype, sizeof(Guid), block, length) is not { } at)
{
return null;
}

subtype = *(Guid*)(block + at);
}

var data = Items(record.cDataItems, record.pDataItems, block, length);

return data is null
? null
: new ServiceTrigger(
ManagerTerms.Trigger(record.dwTriggerType), ManagerTerms.TriggerAction(record.dwAction), subtype, data);
}

/// <summary>The data items of one record, in the manager's order, or null when one cannot be read whole.</summary>
private static unsafe List<TriggerData>? Items(
uint count, SERVICE_TRIGGER_SPECIFIC_DATA_ITEM* items, byte* block, int length)
{
if (count == 0)
{
return [];
}

// The whole array inside the block before any of it is read - a count that does not fit is a
// block that does not describe itself, and the list is not started on a guess.
if (Within(items, (long)count * sizeof(SERVICE_TRIGGER_SPECIFIC_DATA_ITEM), block, length) is not { } at)
{
return null;
}

var records = (SERVICE_TRIGGER_SPECIFIC_DATA_ITEM*)(block + at);
var data = new List<TriggerData>((int)count);

for (uint index = 0; index < count; index++)
{
var item = records[index];

// An item of no bytes points at nothing worth checking - the manager may leave it null.
var from = item.cbData == 0 ? 0 : Within(item.pData, item.cbData, block, length);

if (from is null
|| Value(item.dwDataType, new ReadOnlySpan<byte>(block + from.Value, (int)item.cbData)) is not { } value)
{
return null;
}

data.Add(value);
}

return data;
}

/// <summary>
/// Where a run of bytes the manager pointed at starts inside the block, or null when any byte of
/// it lies outside - a null pointer included, which lands below the block.
/// </summary>
private static unsafe long? Within(void* pointer, long size, byte* block, int length)
{
var offset = (byte*)pointer - block;

return offset >= 0 && offset <= length && size <= length - offset ? offset : null;
}

/// <summary>
/// One data item as text, in the form <see cref="TriggerData"/> describes - or null for a level
/// that is not one byte, a keyword that is not eight, or a string of an odd number of bytes. None
/// of those is a shape the documentation allows, so each is a block that does not describe itself.
/// </summary>
private static TriggerData? Value(SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE type, ReadOnlySpan<byte> bytes) => type switch
{
SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE.SERVICE_TRIGGER_DATA_TYPE_BINARY =>
new TriggerData(TriggerDataType.Binary, [Convert.ToHexStringLower(bytes)]),

SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE.SERVICE_TRIGGER_DATA_TYPE_STRING =>
bytes.Length % sizeof(char) == 0 ? new TriggerData(TriggerDataType.String, Strings(bytes)) : null,

SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE.SERVICE_TRIGGER_DATA_TYPE_LEVEL =>
bytes.Length == 1
? new TriggerData(TriggerDataType.Level, [bytes[0].ToString(CultureInfo.InvariantCulture)])
: null,

SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE.SERVICE_TRIGGER_DATA_TYPE_KEYWORD_ANY => Keyword(TriggerDataType.KeywordAny, bytes),
SERVICE_TRIGGER_SPECIFIC_DATA_ITEM_DATA_TYPE.SERVICE_TRIGGER_DATA_TYPE_KEYWORD_ALL => Keyword(TriggerDataType.KeywordAll, bytes),

// A type the documentation does not name: what is there is kept, and the type says it is not ours.
_ => new TriggerData(TriggerDataType.Unknown, [Convert.ToHexStringLower(bytes)])
};

private static TriggerData? Keyword(TriggerDataType type, ReadOnlySpan<byte> bytes) =>
bytes.Length == sizeof(ulong)
? new TriggerData(type, ["0x" + BinaryPrimitives.ReadUInt64LittleEndian(bytes).ToString("x16", CultureInfo.InvariantCulture)])
: null;

/// <summary>
/// The strings of one item: split at every null, the nulls that close the run taken off the end, and
/// one empty string when nothing is left - so an item always has a value to read.
///
/// <b>Split at every null rather than stopped at the first pair</b>, the way <see cref="ReadMultiString"/>
/// stops: this run has its own length, so an empty string in the middle is something the manager holds
/// rather than the end of the run, and keeping it is what keeps the item whole.
/// </summary>
private static string[] Strings(ReadOnlySpan<byte> bytes)
{
var values = new string(MemoryMarshal.Cast<byte, char>(bytes)).Split('\0');
var kept = values.Length;

while (kept > 1 && values[kept - 1].Length == 0)
{
kept--;
}

return values[..kept];
}
}
Loading
Loading