Merge commit from fork
security: GHSA-jm28-2wcr-qf3h (LOW) enforce logs ACL on sync execution endpoints
This commit is contained in:
James Read
GitHub
commit
e421780c98
|
|
@ -0,0 +1,116 @@
|
|||
# Security Advisory Duplicates — Maintainer Guide
|
||||
|
||||
This document lists known duplicate security advisory clusters for [OliveTin/OliveTin](https://github.com/OliveTin/OliveTin). When triaging new reports, check here and [open advisories](https://github.com/OliveTin/OliveTin/security/advisories) before accepting.
|
||||
|
||||
**Duplicate policy:** the earliest reporter on the canonical advisory receives primary credit. Later reporters are credited on the canonical advisory when closed as duplicates. See [SECURITY.md](../SECURITY.md).
|
||||
|
||||
## Triage checklist
|
||||
|
||||
1. Search open advisories for the same component and attack path.
|
||||
2. Match against clusters below.
|
||||
3. If duplicate: close the newer advisory, link to canonical, add reporter to canonical credits.
|
||||
4. If unique: accept, patch on a private branch, reassess CVSS with OliveTin context (see SECURITY.md — OliveTin is intentional RCE by design).
|
||||
5. Merge fix to `next`, publish advisory, credit reporters in advisory body (not commit message).
|
||||
|
||||
---
|
||||
|
||||
## shellAfterCompleted command injection
|
||||
|
||||
**Canonical:** [GHSA-vc6p-m6vx-6cwq](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-vc6p-m6vx-6cwq) — reporter **knight-yagami** (2026-03-04)
|
||||
|
||||
Untrusted command output or template variables interpolated into `shellAfterCompleted` and executed via `sh -c`.
|
||||
|
||||
| GHSA | Reporter | Status | Notes |
|
||||
|------|----------|--------|-------|
|
||||
| GHSA-vc6p-m6vx-6cwq | knight-yagami | closed (canonical) | Original report |
|
||||
| GHSA-v5gc-hqpq-227p | 0xkakash1 | closed (duplicate) | Output template variant |
|
||||
| GHSA-m7wr-wj5j-7459 | Ryu7zz | duplicate | Webhook `exec` → output → `shellAfterCompleted` |
|
||||
| GHSA-cjxm-x848-6vmc | Yesuhei | duplicate | Missing shell safety on after-completion |
|
||||
| GHSA-j9p9-36jc-2v8w | anushkavirgaonkar | duplicate | Same root cause |
|
||||
|
||||
**Fix:** shell-quote `output`/`exitCode` before template render; block `shellAfterCompleted` for webhook-tagged actions.
|
||||
|
||||
**CVSS note:** requires admin-configured `shellAfterCompleted` and attacker influence on output — typically PR:H not PR:N.
|
||||
|
||||
---
|
||||
|
||||
## OAuth2 state map memory exhaustion (DoS)
|
||||
|
||||
**Canonical:** [GHSA-xpxj-f2fm-rqch](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xpxj-f2fm-rqch) — reporter **knight-yagami** (2026-03-04)
|
||||
|
||||
Unauthenticated `/oauth/login` grows `registeredStates` without TTL or cap.
|
||||
|
||||
| GHSA | Reporter | Status | Notes |
|
||||
|------|----------|--------|-------|
|
||||
| GHSA-xpxj-f2fm-rqch | knight-yagami | closed (canonical) | Original report |
|
||||
| GHSA-cj96-c55v-2f3c | Dredsen | duplicate | Same unbounded map |
|
||||
|
||||
**Fix:** TTL sweep (match 15-minute cookie MaxAge), max map size, cleanup on failed callback.
|
||||
|
||||
**CVSS note:** unauthenticated DoS — reported 7.5 is appropriate.
|
||||
|
||||
---
|
||||
|
||||
## URL argument type — unrestricted URI schemes (SSRF / file read)
|
||||
|
||||
**Canonical:** [GHSA-45pc-w4ph-hrq4](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-45pc-w4ph-hrq4) — reporter **fg0x0** (2026-03-09)
|
||||
|
||||
`url` type accepts `file://`, `gopher://`, etc. Blocked in `shell:` mode but still validated weakly for `exec:` actions.
|
||||
|
||||
| GHSA | Reporter | Status | Notes |
|
||||
|------|----------|--------|-------|
|
||||
| GHSA-45pc-w4ph-hrq4 | fg0x0 | closed (canonical) | Original report |
|
||||
| GHSA-cchg-25m4-q6rj | anushkavirgaonkar | duplicate | Same scheme validation gap |
|
||||
|
||||
**Fix:** allowlist `http`/`https` in `typeSafetyCheckUrl`.
|
||||
|
||||
**CVSS note:** admin must configure `exec:` action passing URL to external tool — PR:H.
|
||||
|
||||
---
|
||||
|
||||
## Custom `regex:` argument type in shell actions
|
||||
|
||||
**Canonical:** [GHSA-xc5w-4v5w-7x65](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xc5w-4v5w-7x65) — reporter **Ayantaker** (2026-05-06)
|
||||
|
||||
`regex:` types not in shell denylist; partial `MatchString` allows injection suffixes.
|
||||
|
||||
| GHSA | Reporter | Status | Notes |
|
||||
|------|----------|--------|-------|
|
||||
| GHSA-xc5w-4v5w-7x65 | Ayantaker | canonical | Missing denylist entry |
|
||||
| GHSA-gvxq-7gvp-4ggr | anushkavirgaonkar | duplicate | Unanchored partial match |
|
||||
|
||||
**Fix:** deny `regex:` in shell mode; enforce full-string match for custom regex types.
|
||||
|
||||
---
|
||||
|
||||
## Shell denylist incomplete (post CVE-2026-27626)
|
||||
|
||||
**Canonical:** [GHSA-c26w-h42g-jfp9](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-c26w-h42g-jfp9) — reporter **sec-reex** (2026-07-03)
|
||||
|
||||
CVE-2026-27626 added `password` to denylist only; `html`, `confirmation`, and choiceless `checkbox` still skip validation and are allowed in `shell:` actions.
|
||||
|
||||
**Fix:** extend `checkShellArgumentSafety` denylist.
|
||||
|
||||
---
|
||||
|
||||
## StartActionAndWait logs ACL bypass
|
||||
|
||||
**Canonical:** [GHSA-jm28-2wcr-qf3h](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-jm28-2wcr-qf3h) — reporter **offset** (2026-03-12)
|
||||
|
||||
`StartActionAndWait` / `StartActionByGetAndWait` return full log output without `logs` ACL check.
|
||||
|
||||
No known duplicates.
|
||||
|
||||
**Fix:** apply `isLogEntryAllowed` before returning `LogEntry`.
|
||||
|
||||
**CVSS note:** requires authenticated user with `exec` but not `logs` — typically 4.3–5.3 not 6.5.
|
||||
|
||||
---
|
||||
|
||||
## Easy to confuse (not duplicates)
|
||||
|
||||
| Topic | Advisories | Distinction |
|
||||
|-------|------------|-------------|
|
||||
| OAuth2 state DoS vs OAuth2 auth bypass | GHSA-xpxj vs GHSA-3v7p | DoS fills state map; bypass spoofs `authHttpHeaderUsername` |
|
||||
| `shellAfterCompleted` vs direct `shell` injection | GHSA-vc6p vs GHSA-49gm | Second-order via output vs first-order argument injection |
|
||||
| `ValidateArgumentType` enumeration | GHSA-f637 vs GHSA-x6q3 | Same issue; GHSA-f637 published |
|
||||
|
|
@ -282,6 +282,21 @@ func (api *oliveTinAPI) findBindingByIDOrNotFound(bindingId string) (*executor.A
|
|||
return api.findBindingOrNotFound(bindingId)
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) startActionAndWaitLogEntry(binding *executor.ActionBinding, args map[string]string, justification string, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
|
||||
internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user)
|
||||
if !ok {
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
|
||||
}
|
||||
return api.logEntryForAllowedViewer(internalLogEntry, user)
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) logEntryForAllowedViewer(internalLogEntry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
|
||||
if err := api.requireLogEntryAllowed(internalLogEntry, user); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return api.internalLogEntryToPb(internalLogEntry, user), nil
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionAndWaitRequest]) (*connect.Response[apiv1.StartActionAndWaitResponse], error) {
|
||||
binding, err := api.findBindingOrNotFound(req.Msg.ActionId)
|
||||
if err != nil {
|
||||
|
|
@ -295,12 +310,12 @@ func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request
|
|||
return nil, connectInvalidJustification(err)
|
||||
}
|
||||
|
||||
internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user)
|
||||
if !ok {
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
|
||||
logEntry, err := api.startActionAndWaitLogEntry(binding, args, justification, user)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return connect.NewResponse(&apiv1.StartActionAndWaitResponse{
|
||||
LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
|
||||
LogEntry: logEntry,
|
||||
}), nil
|
||||
}
|
||||
|
||||
|
|
@ -327,16 +342,7 @@ func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *connect.Request[a
|
|||
}), nil
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) {
|
||||
binding := api.executor.FindBindingByID(req.Msg.ActionId)
|
||||
if binding == nil || binding.Action == nil {
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
|
||||
}
|
||||
|
||||
args := make(map[string]string)
|
||||
|
||||
user := auth.UserFromApiCall(ctx, req, api.cfg)
|
||||
|
||||
func (api *oliveTinAPI) runBindingAndWait(binding *executor.ActionBinding, args map[string]string, user *authpublic.AuthenticatedUser) (*executor.InternalLogEntry, bool) {
|
||||
execReq := executor.ExecutionRequest{
|
||||
Binding: binding,
|
||||
TrackingID: uuid.NewString(),
|
||||
|
|
@ -348,15 +354,32 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Re
|
|||
wg, _ := api.executor.ExecRequest(&execReq)
|
||||
wg.Wait()
|
||||
|
||||
internalLogEntry, ok := api.executor.GetLog(execReq.TrackingID)
|
||||
|
||||
if ok {
|
||||
return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{
|
||||
LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
|
||||
}), nil
|
||||
return api.executor.GetLog(execReq.TrackingID)
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) startActionByGetAndWaitLogEntry(binding *executor.ActionBinding, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
|
||||
internalLogEntry, ok := api.runBindingAndWait(binding, map[string]string{}, user)
|
||||
if !ok {
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
|
||||
}
|
||||
return api.logEntryForAllowedViewer(internalLogEntry, user)
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) {
|
||||
binding := api.executor.FindBindingByID(req.Msg.ActionId)
|
||||
if binding == nil || binding.Action == nil {
|
||||
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
|
||||
}
|
||||
|
||||
user := auth.UserFromApiCall(ctx, req, api.cfg)
|
||||
logEntry, err := api.startActionByGetAndWaitLogEntry(binding, user)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{
|
||||
LogEntry: logEntry,
|
||||
}), nil
|
||||
}
|
||||
|
||||
func calculateRateLimitExpires(api *oliveTinAPI, logEntry *executor.InternalLogEntry) string {
|
||||
if logEntry.Binding == nil || logEntry.Binding.Action == nil {
|
||||
|
|
@ -684,6 +707,13 @@ func (api *oliveTinAPI) isLogEntryAllowed(e *executor.InternalLogEntry, user *au
|
|||
return acl.IsAllowedLogs(api.cfg, user, e.Binding.Action)
|
||||
}
|
||||
|
||||
func (api *oliveTinAPI) requireLogEntryAllowed(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) error {
|
||||
if api.isLogEntryAllowed(entry, user) {
|
||||
return nil
|
||||
}
|
||||
return connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied to view this execution"))
|
||||
}
|
||||
|
||||
// mayViewExecutionEvent returns whether the user is allowed to receive this execution event (for EventStream ACL).
|
||||
func (api *oliveTinAPI) mayViewExecutionEvent(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) bool {
|
||||
if user == nil {
|
||||
|
|
|
|||
|
|
@ -385,6 +385,54 @@ func testWithEntity(t *testing.T, binding *executor.ActionBinding, rr *Dashboard
|
|||
assert.Equal(t, expectedCanExec, actionResult.CanExec, message)
|
||||
}
|
||||
|
||||
// buildExecWithoutLogsTestConfig returns config for GHSA-jm28: user "runner" may exec but not read logs.
|
||||
func buildExecWithoutLogsTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser) {
|
||||
t.Helper()
|
||||
cfg := config.DefaultConfig()
|
||||
cfg.AuthHttpHeaderUsername = "X-Ot-User"
|
||||
cfg.DefaultPermissions.View = false
|
||||
cfg.DefaultPermissions.Exec = false
|
||||
cfg.DefaultPermissions.Logs = false
|
||||
|
||||
cfg.Actions = append(cfg.Actions, &config.Action{
|
||||
ID: "run_only",
|
||||
Title: "Run Only",
|
||||
Shell: "echo sensitive-output",
|
||||
Icon: "🔒",
|
||||
})
|
||||
|
||||
cfg.AccessControlLists = append(cfg.AccessControlLists, &config.AccessControlList{
|
||||
Name: "runner",
|
||||
MatchUsernames: []string{"runner"},
|
||||
AddToEveryAction: true,
|
||||
Permissions: config.PermissionsList{View: true, Exec: true, Logs: false, Kill: false},
|
||||
})
|
||||
|
||||
runner := &authpublic.AuthenticatedUser{Username: "runner"}
|
||||
runner.BuildUserAcls(cfg)
|
||||
return cfg, runner
|
||||
}
|
||||
|
||||
// TestStartActionAndWaitDeniesLogsPermission (GHSA-jm28-2wcr-qf3h) asserts sync execution endpoints
|
||||
// enforce logs ACL and do not return action output to users allowed to exec but not read logs.
|
||||
func TestStartActionAndWaitDeniesLogsPermission(t *testing.T) {
|
||||
cfg, _ := buildExecWithoutLogsTestConfig(t)
|
||||
ex := executor.DefaultExecutor(cfg)
|
||||
ex.RebuildActionMap()
|
||||
ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
|
||||
defer ts.Close()
|
||||
|
||||
req := connect.NewRequest(&apiv1.StartActionAndWaitRequest{
|
||||
ActionId: "run_only",
|
||||
})
|
||||
req.Header().Set("X-Ot-User", "runner")
|
||||
|
||||
_, err := client.StartActionAndWait(context.Background(), req)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
|
||||
"user with exec:true and logs:false must not receive log output from StartActionAndWait")
|
||||
}
|
||||
|
||||
// buildViewPermissionTestConfig returns config and users for GHSA view-permission tests:
|
||||
// one action "secret_action", ACL "restricted" (view:false, logs:false) for user "low", ACL "full" (view:true, logs:true) for user "admin".
|
||||
func buildViewPermissionTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser, *authpublic.AuthenticatedUser) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue