Audit event streaming JSON schemas use ^/$ anchors, letting multi-line values bypass format validation
The following discussion from !249803 (merged) should be addressed:
-
@rbarnwal1 started a discussion: (+1 comment)
note (non-blocking) Pre-existing issue, not this MR's fault, but
bucketName(^[a-z0-9][a-z0-9\-.]*$) andaccessKeyXid(^[a-zA-Z0-9_]+$) in this same schema file still use^/$anchors instead of\A/\z, so with json_schemer's Ruby regexp resolver, something like"validbucket\nevil"would pass today. Same bug class this MR is fixing forawsRegion. The legacy models' own Ruby regexes for both fields already use\A/\z, it's only the schema path that's affected. You could either align those two here since you're already in this file, or open a follow-up issue and tackle it separately. wdyt?
Audit event streaming JSON schemas use ^/$ anchors, letting multi-line values bypass format validation
Summary
JSON schema pattern constraints in the audit event streaming config schemas are anchored with ^/$. json_schemer compiles a pattern with the default regexp_resolver of "ruby", which means the pattern becomes a plain Regexp and ^/$ bind to line boundaries rather than string boundaries.
The result is that a value only has to satisfy the pattern on one of its lines. Anything after a newline is unvalidated.
This was found and fixed for awsRegion in !249803 (merged)(see #608459 (closed)). The same bug class remains on the sibling fields.
Affected fields
Verified against the current schemas:
| Schema | Field | Pattern | "valid\nevil" accepted? |
|---|---|---|---|
| aws | accessKeyXid |
^[a-zA-Z0-9_]+$ |
yes |
| aws | bucketName |
^[a-z0-9][a-z0-9\-.]*$ |
yes |
| aws | awsRegion |
\A...\z |
no — already fixed |
Reproduction:
require 'json_schemer'
schema = JSONSchemer.schema(
Rails.root.join('ee/app/validators/json_schemas',
'audit_events_aws_external_streaming_destination_config.json')
)
schema.valid?({
'accessKeyXid' => 'AAAAAAAAAAAAAAAA',
'bucketName' => "validbucket\nevil",
'awsRegion' => 'us-east-1'
})
# => trueAlso worth reviewing in the same pass, though not the same defect: audit_events_http_external_streaming_destination_config.json anchors url with ^(https?://.+|\$[A-Za-z]+) and no closing anchor, so it is a prefix match; and the header key/value patterns use ^/$.
Why it matters
For the current ExternallyStreamable destination models, the JSON schema is the only format guard on these fields — there is no Ruby-level regexp behind it. The legacy AmazonS3Configuration and GoogleCloudLoggingConfiguration models are not affected, because their own validations already use \A/\z.
Values reach outbound requests: bucketName and accessKeyXid are passed to Aws::S3::Client (the access key ID participates in SigV4 signing), and logIdName is interpolated into full_log_path for the Google Cloud Logging API. Embedded newlines in values used to build requests are a known injection shape, so this is worth an AppSec opinion even though no exploit has been demonstrated and the AWS/Google clients may reject or escape such values themselves.
Severity is limited by who can reach it: only group Owners and instance administrators can configure streaming destinations, so this is a validation bypass for someone who already holds that role, not a privilege escalation.
Proposed fix
Change the affected patterns to \A/\z:
- "pattern": "^[a-zA-Z0-9_]+$",
+ "pattern": "\\A[a-zA-Z0-9_]+\\z",Two things to know before implementing:
- Do not use a
(?!.*[\r\n])lookahead. That approach is used inapp/validators/json_schemas/web_hooks_custom_headers.json, and it does not close the gap — the regexp engine can still begin matching on a later line. Verified.\A/\zis the form that holds. - This is a new validation on existing data. Rows may already store values with embedded newlines, and these validations re-run on every save, so tightening them can block unrelated edits (including deactivating a destination, since
Activatable#deactivate!isupdate!(active: false)). Follow the code quality guidance on making a new validation optional on existing data — theawsRegionfix gates onnew_record? || will_save_change_to_aws_region?for exactly this reason.
Wider context, out of scope here
awsRegion is currently the only \A-anchored pattern in the repository:
app/validators/json_schemas + ee/app/validators/json_schemas
files with ^/$ anchored patterns : 35
files with \A anchored patterns : 0So this is a repo-wide convention rather than a local mistake, and most of those 35 files are probably fine because a Ruby-level validation sits behind them. This issue is scoped to the audit event streaming schemas. A broader audit, or a shared spec helper that flags ^/$ in any pattern whose schema is the sole guard, would be a reasonable separate follow-up.
Acceptance criteria
-
accessKeyXidandbucketNamein the aws schema anchored with\A/\z -
googleProjectIdNameandlogIdNamein the gcp schema anchored with\A/\z - Decision recorded on the http schema's
urland header patterns - Specs cover a multi-line value per field, asserting rejection
- New format validations are gated so existing rows can still be deactivated and renamed
- Query run to find rows already storing multi-line values in these fields