TD-01 — Published contract bound to the framework default namespace http://tempuri.org/ |
clsADUserServices.asmx.vb line 38 (<System.Web.Services.WebService(Namespace:="http://tempuri.org/")>) |
The WSDL target namespace and every message/SOAP-action namespace collide with any other service that never changed the default; changing it later breaks every generated consumer proxy already in the field |
Assign a stable, organization-specific namespace before any further consumer onboarding; treat the change as a breaking-contract migration for existing consumers |
TD-18 — AddUser continues after a caught userPrin.Save() failure, operating on an unsaved principal |
clsADUserServices.AddUser, lines 145–152 |
A save failure sets Response = Failure but the method proceeds into the directory-attribute block against a principal that was never persisted, risking inconsistent partial state |
Return immediately after a Save() failure instead of falling through to the attribute-write block |
TD-19 — ChangeUserAccount changes to company/info never set the modified flag, so CommitChanges() is skipped silently while the operation returns Success |
clsADUserServices.ChangeUserAccount, lines 435–457 |
A caller believes an attribute change succeeded when nothing was committed to the directory |
Set modified = True for every directory-level attribute assignment, not only the principal-level ones |
TD-20 — DeleteUserFromGroup's Catch block is a deliberate swallow ('gobble), so any directory failure returns Success |
clsADUserServices.DeleteUserFromGroup, lines 331–333 |
A genuine directory failure is indistinguishable from a completed group removal |
Distinguish "user not a member" from a genuine exception; return a failure sentinel for the latter |
TD-21 — GetUserGroups/GetUserResources empty Catch blocks return an empty ArrayList on any failure |
clsADUserServices.asmx.vb lines 574–576, 610–612 |
A directory outage is indistinguishable from "user has no groups/resources" to every caller |
Surface a distinct failure signal instead of swallowing the exception into an empty result |
TD-27 — ArtifactNameExternal is set to app-internal, the same value as ArtifactNameInternal, so both build variants publish to one artifact name |
pipelines\build-vars.yml |
The two deployment variants cannot be released as distinguishable artifacts; a consumer of build history cannot tell which variant an artifact came from |
Assign a distinct artifact name to the External variant |
SEC-01 — IIS anonymous authentication enabled for both deployment variants while Web.config declares Windows authentication and defines no <authorization> element |
pipelines\release.yml (IISAnonymousAuthInternal: true, IISAnonymousAuthExternal: true); src\ADUserServices\Web.config |
The declared Windows-authentication mode is not enforced; the only effective barrier on the mutating surface is the IP allow-list |
Add an explicit <authorization> policy, or confirm and document that anonymous access combined with the allow-list is the intended posture |
| SEC-02 — Eleven directory-reading operations carry no application-layer authorization, and nine of them disclose real Active Directory data |
clsADUserServices.asmx.vb — DoesUserExist, DoesUserHaveResource, GetUserEmail, GetUserGroups, GetUserResources, GetUserFAX, GetUserLastLogon, GetUserPrinter, IsUserInGroup (plus HelloWorld, CheckAccess with no data disclosure) |
Any caller that can reach the endpoint can read AD attributes and group/resource membership across both the internal and external domains |
Apply the same CheckAccess gate, or an equivalent authorization construct, to the disclosing read operations |
SEC-03 — The allow-list trusts HTTP_X_FORWARDED_FOR in preference to REMOTE_ADDR, with no trusted-proxy validation and no multi-hop parsing |
clsADUserServices.GetIncomingIp, lines 798–806, consumed by CheckAccess lines 752–776 |
A caller able to set that header can present an arbitrary source address and pass the allow-list check |
Validate the header only from a known reverse proxy, or rely on REMOTE_ADDR |
SEC-06 — Plaintext passwords are accepted as SOAP string parameters (AddUser.inPassword, ChangeUserPassword/ChangeUserPasswordOrGetException's inNewPassword), with no transport security evidenced in the repository |
clsADUserServices.asmx.vb — AddUser, ChangeUserPassword, ChangeUserPasswordOrGetException, private ChangePassword |
A caller-supplied password travels in the request body with protection dependent entirely on a transport binding this repository does not declare |
Require TLS at the endpoint and confirm the deployed binding enforces it |
| SEC-08 — A full directory-administration surface (create, modify, delete, unlock, re-password, re-group) sits behind a single IP allow-list with no user identity, no entitlement model, and no audit trail |
clsADUserServices.asmx.vb, all eight mutating operations |
Every privileged action is attributed to one shared app-pool identity with no way to trace which caller performed it |
Introduce caller identity and audit logging in addition to the network-origin gate |
SEC-16 — AddUserToGroup/DeleteUserFromGroup allow unconstrained group-membership changes, with no allow-list, name-pattern restriction, or exclusion of privileged/*Resource* groups |
clsADUserServices.AddUserToGroup, DeleteUserFromGroup |
A complete account-creation-to-entitlement escalation chain exists within one endpoint, gated only by network origin — an account created with the default password (SEC-05, CRITICAL) can be added to any resolvable group |
Constrain which groups may be targeted by these operations, particularly privileged and *Resource* groups |
SEC-17 — DoesUserHaveResource (transitive, via GetAuthorizationGroups) and IsUserInGroup (direct membership only, via Members.Contains) answer the same kind of authorization question inconsistently |
clsADUserServices.DoesUserHaveResource, IsUserInGroup, GetGroupPrincipal |
A consumer relying on one oracle over the other receives a different authorization answer for the same user/group, and both oracles are ungated |
Reconcile the two operations on a single membership semantic, or document the difference prominently on the contract |
SEC-19 — Password operations (ChangeUserPassword, ChangeUserPasswordOrGetException) are resets, not changes: no current-password verification, and no complexity/history validation in application code |
clsADUserServices.ChangePassword |
Any allow-listed origin can set the password of any external-domain account without proving knowledge of the existing one |
Require current-password verification or an equivalent step-up check before resetting |
SEC-12 — The 13-track allowedips allow-list is committed in cleartext (environmentsettings\settings.yml) alongside the spoofable HTTP_X_FORWARDED_FOR check that consumes it |
environmentsettings\settings.yml, consumed by clsADUserServices.GetAllowedIPs |
In combination with SEC-03, the repository publishes the exact HTTP_X_FORWARDED_FOR values an attacker needs to pass the allow-list for every environment; the storage mechanism itself is appropriate for a non-secret setting, so the finding is the composition, not the storage |
Address the composite risk primarily by fixing SEC-03 (the spoofable header preference); consider whether publishing the full per-environment list is necessary |
| SEC-14 — No requester attribution on any directory mutation; every account creation, deletion, attribute change, password reset, unlock, and group-membership change executes as one shared app-pool identity |
clsADUserServices — all directory helpers and all mutating operations |
Directory-side audit records cannot distinguish which caller, environment, or origin performed a given mutation, and this is compounded by the same service account being used for non-prod and prod (SEC-07) |
Introduce caller identity propagation and correlate it with directory-side audit logs |
SEC-15 — Nine ungated operations (GetUserEmail, GetUserFAX, GetUserPrinter, GetUserLastLogon, GetUserGroups, GetUserResources, IsUserInGroup, DoesUserExist, DoesUserHaveResource) accept a caller-supplied inUserId and domain flag with no authentication, exposing both the internal (corp.saif.com) and external forests to unauthenticated enumeration |
clsADUserServices — the nine ungated directory operations |
Any unauthenticated caller can enumerate user existence, contact/attribute data, and — via GetUserGroups — the complete transitive authorization-group list for any named user across both forests |
Apply the same CheckAccess gate, or an equivalent authorization construct, to these operations |