Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.1.0h -> lost quotes in c_rehash #5772

Closed
sebastianas opened this issue Mar 28, 2018 · 9 comments
Closed

1.1.0h -> lost quotes in c_rehash #5772

sebastianas opened this issue Mar 28, 2018 · 9 comments

Comments

@sebastianas
Copy link
Contributor

Since #5533 the c_rehash here has no quotes which were specified in "quotify1":

my $dir = {- quotify1($config{openssldir}) -};
my $prefix = {- quotify1($config{prefix}) -};

See Debian bug 894282. I reverted commit
77ba00b ("util/dofile.pl: only quote stuff that actually needs quoting")

to around it. Any idea?

Sebastian

@jrtc27
Copy link

jrtc27 commented Mar 28, 2018

Yeah, it's horribly broken. The backslash-escaping only escapes things which need escaping inside double quotes, and assumes that anything which doesn't need any backslashes also doesn't need quote, but that's not true. Forward slashes don't need to be escaped, but either mean a search pattern or division, based on whether they are at the start of the string. I don't know the full perl grammar, but it seems to me that, if you want to avoid double quotes when not necessary (which seems like a waste of time that causes needless problems like this, if you ask me, but that's not my call), that should be whitelisted with something like [A-Za-z0-9_]+ or whatever the grammar specifies.

@levitte
Copy link
Member

levitte commented Mar 28, 2018

Side note: the given URL for the Debian bug report didn't work for me, but this does: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=894282

@levitte
Copy link
Member

levitte commented Mar 28, 2018

Thank you for the report, btw. Will fix.

@jrtc27
Copy link

jrtc27 commented Mar 28, 2018

Yeah, it's either bugs.d.o/BUGNUM or bugs.d.o/cgi-bin/bugreport.cgi?bug=BUGNUM, not bugs.d.o/cgi-bin/BUGNUM.

@levitte
Copy link
Member

levitte commented Mar 28, 2018

I did, however, make that change for a reason... Unfortunately, I can only remember vaguely something about an empty argument that shouldn't have been there because some perl array contained an undef or something like that.

Anyway, I'm reverting that change and seeing what breaks because of it. Be right back...

levitte added a commit to levitte/openssl that referenced this issue Mar 28, 2018
This wasn't a good solution, too many things depend on the quotes being
there consistently.

This reverts commit 49cd47e.

Fixes openssl#5772
@levitte
Copy link
Member

levitte commented Mar 28, 2018

Side note: while I understand the nature of habit, I would urge those who can (and this most definitely includes Linux as far as I know) to switch to use openssl rehash. c_rehash is a kinda fallback script that will disappear at some point.

@CyrilBrulebois
Copy link

Regarding the side note: I'm seeing fewer symlinks generated by openssl rehash compared to c_rehash… Is there any documentation regarding your recommendation to switch and the possible side effects?

@kroeckx
Copy link
Member

kroeckx commented Mar 28, 2018 via email

levitte added a commit that referenced this issue Mar 29, 2018
This wasn't a good solution, too many things depend on the quotes being
there consistently.

This reverts commit 49cd47e.

Fixes #5772

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5773)

(cherry picked from commit 00701e5)
@sebastianas
Copy link
Contributor Author

sebastianas commented Apr 3, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants