On 15.11.23 15:49, Thomas Lamprecht wrote: > Am 15/11/2023 um 14:28 schrieb Stefan Sterz: >> On 15.11.23 13:23, Markus Frank wrote:
-- >8 snip 8< -- >> >>> src/PVE/JSONSchema.pm | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm >>> index 49e0d7a..ef58b62 100644 >>> --- a/src/PVE/JSONSchema.pm >>> +++ b/src/PVE/JSONSchema.pm >>> @@ -408,7 +408,7 @@ PVE::JSONSchema::register_format('ldap-simple-attr', >>> \&verify_ldap_simple_attr); >>> sub verify_ldap_simple_attr { >>> my ($attr, $noerr) = @_; >>> >>> - if ($attr =~ m/^[a-zA-Z0-9]+$/) { >>> + if ($attr =~ m/^[a-zA-Z0-9\-]+$/) { >> >> if i'm not mistaken, this regex should try to filter an `AttributeValue` >> [1]. in case we do stick with this regex approach here, you may want to >> relax this even further, as per the standard: >> >>> If that UTF-8-encoded Unicode string does not have any of the >>> following characters that need escaping, then that string can be used >>> as the string representation >>> of the value. >>> >>> - a space (' ' U+0020) or number sign ('#' U+0023) occurring at >>> the beginning of the string; >>> >>> - a space (' ' U+0020) character occurring at the end of the >>> string; >>> >>> - one of the characters '"', '+', ',', ';', '<', '>', or '\' >>> (U+0022, U+002B, U+002C, U+003B, U+003C, U+003E, or U+005C, >>> respectively); >>> >>> - the null (U+0000) character. >>> > > Ack, so I was wrong, the format might still make sense albeit checking > for above cases would then indeed better, something along the lines of: > > if ($attr !~ /(?:^(?:\s|#))|["+,;<>\0\\]|(?:\s$)/) { > return $attr; > } > > If we leave that regex out completely we should ensure that we don't get > any tainting issues. > > The format could move to PVE::Auth::LDAP too, FWIW, but that's a different > story. just to through this out there, my last attempt at validating this [1] looked something like this: ``` my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!; my $start = qr!(?:${escaped}|[^"+,;<>\\\0 #])!; my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!; my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!; my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!; ``` since things can also be escaped or in quotes, which makes them valid again. could probably be improved here, though. [1]: https://lists.proxmox.com/pipermail/pve-devel/2023-May/056840.html _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel