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

PCRE expressions with capture groups are not being handled properly #1300

Closed
mdell-seradex opened this issue Aug 13, 2021 · 17 comments
Closed
Assignees

Comments

@mdell-seradex
Copy link

mdell-seradex commented Aug 13, 2021

What was done

We are setting up a new ftp server because we need to move it off site for dumb reasons IMHO.
I used the same rewrite entries that are currently working in the 1.3.6c version, but cannot get it to work.
I enabled all kinds of logging and see something that seems peculiar in the trace logs.

On the v1.3.6c server I see this:

<regexp:9>: executing POSIX regex '(.*)' against subject '/folder1\folder2'
<regexp:9>: executing PCRE regex 'RETR|SIZE|LIST|CWD' against subject 'CWD'
<encode:5>: decoded 'CWD' into 'CWD'
<encode:5>: decoded '/folder1/folder2' into '/folder1/folder2'

On the v1.3.7a server I see this instead (where it does not work):

<regexp:9>: executing PCRE regex '(.*)' against subject '/folder1\folder2'
<regexp:9>: executing PCRE regex 'RETR|SIZE|LIST|CWD' against subject 'CWD'
<command:7>: dispatching PRE_CMD command 'CWD /folder1\folder2' to mod_core.c
<command:7>: dispatching PRE_CMD command 'CWD /folder1\folder2' to mod_core.c
<regexp:9>: executing PCRE regex '\*.*/' against subject '/folder1\folder2'
<regexp:9>: PCRE regex '\*.*/' failed to match subject '/folder1\folder2': subject did not match pattern

Notice that the new server is not mentioning POSIX. This makes me think I need to somehow enable POSIX support. I hope it just requires a minor change to the proftpd.config files, or the installation of a missing package as opposed to an issue with the proftpd package available for this version of Ubuntu.
Note that I checked to see if a new version of the proftpd package was available, but it did not find any.

**Also decoded '/folder1/folder2' into '/folder1/folder2' seems wrong. I would think it should be decoded '/folder1\folder2' into '/folder1/folder2'.

ProFTPD Version and Configuration

ProFTPD Version 1.3.7a installed on Ubuntu 21.04 (GNU/Linux 5.11.0-25-generic x86_64)
It appears to be the "proftpd-core" package.

Output from proftpd -V:

ProFTPD Version 1.3.7a
root@localhost:/etc/proftpd# proftpd -V
Compile-time Settings:
  Version: 1.3.7a (maint)
  Platform: LINUX [Linux 5.11.0-25-generic x86_64]
  Built: Fri Jan 15 2021 14:09:32 UTC
  Built With:
    configure  '--infodir=/share/info' '--disable-option-checking' '--disable-silent-rules' '--libdir=/lib/x86_64-linux-gnu' '--disable-dependency-tracking' '--prefix=/usr' '--with-pkgconfig=lib/pkgconfig' '--with-includes=/usr/include/postgresql:/usr/include/mysql' '--mandir=/usr/share/man' '--sysconfdir=/etc/proftpd' '--localstatedir=/run' '--libexecdir=/usr/lib/proftpd' '--enable-sendfile' '--enable-facl' '--enable-dso' '--enable-autoshadow' '--enable-ctrls' '--enable-openssl' '--enable-ipv6' '--enable-nls' '--enable-memcache' '--with-lastlog=/var/log/lastlog' '--enable-pcre' '--disable-strip' '--enable-redis' '--build' 'x86_64-linux-gnu' '--with-shared=mod_unique_id:mod_site_misc:mod_load:mod_ban:mod_quotatab:mod_sql:mod_sql_mysql:mod_sql_postgres:mod_sql_sqlite:mod_sql_odbc:mod_dynmasq:mod_quotatab_sql:mod_ldap:mod_quotatab_ldap:mod_ratio:mod_tls:mod_rewrite:mod_radius:mod_wrap:mod_wrap2:mod_wrap2_file:mod_wrap2_sql:mod_quotatab_file:mod_quotatab_radius:mod_facl:mod_ctrls_admin:mod_copy:mod_deflate:mod_ifversion:mod_geoip:mod_exec:mod_sftp:mod_sftp_pam:mod_sftp_sql:mod_shaper:mod_sql_passwd:mod_ifsession:mod_auth_otp:mod_tls_redis:mod_wrap2_redis:mod_redis:mod_memcache:mod_tls_memcache:mod_readme:mod_snmp:mod_digest:mod_ident:mod_log_forensic:mod_qos:mod_statcache:mod_tls_fscache:mod_tls_shmcache:mod_dnsbl' 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -ffile-prefix-map=/build/proftpd-dfsg-1dlmzn/proftpd-dfsg-1.3.7a+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now' 'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2' 'CXXFLAGS=-g -O2 -ffile-prefix-map=/build/proftpd-dfsg-1dlmzn/proftpd-dfsg-1.3.7a+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security'

  CFLAGS: -g2 -g -O2 -ffile-prefix-map=/build/proftpd-dfsg-1dlmzn/proftpd-dfsg-1.3.7a+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security -Wall -fno-omit-frame-pointer -Werror=implicit-function-declaration
  LDFLAGS: -L$(top_srcdir)/lib -L$(top_builddir)/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now -rdynamic  -L/usr/lib/x86_64-linux-gnu -L/usr/lib/x86_64-linux-gnu
  LIBS: -lacl  -lpcreposix -lpcre -lssl -lcrypto -lsodium -lcap  -lpam -lsupp -lattr -lnsl -lresolv -lresolv -lcrypt -ldl -lhiredis -lmemcachedutil -lmemcached  -pthread

  Files:
    Configuration File:
      /etc/proftpd/proftpd.conf
    Pid File:
      /run/proftpd.pid
    Scoreboard File:
      /run/proftpd.scoreboard
    Header Directory:
      /usr/include/proftpd
    Shared Module Directory:
      /usr/lib/proftpd

  Info:
    + Max supported UID: 4294967295
    + Max supported GID: 4294967295

  Features:
    + Autoshadow support
    + Controls support
    + curses support
    - Developer support
    + DSO support
    + IPv6 support
    + Largefile support
    + Lastlog support
    + Memcache support
    + ncursesw support
    + NLS support
    + OpenSSL support (OpenSSL 1.1.1f  31 Mar 2020)
    + PCRE support
    + POSIX ACL support
    + Redis support
    + Sendfile support
    + Shadow file support
    + Sodium support
    + Trace support
    + xattr support

  Tunable Options:
    PR_TUNABLE_BUFFER_SIZE = 1024
    PR_TUNABLE_DEFAULT_RCVBUFSZ = 8192
    PR_TUNABLE_DEFAULT_SNDBUFSZ = 8192
    PR_TUNABLE_ENV_MAX = 2048
    PR_TUNABLE_GLOBBING_MAX_MATCHES = 100000
    PR_TUNABLE_GLOBBING_MAX_RECURSION = 8
    PR_TUNABLE_HASH_TABLE_SIZE = 40
    PR_TUNABLE_LOGIN_MAX = 256
    PR_TUNABLE_NEW_POOL_SIZE = 512
    PR_TUNABLE_PATH_MAX = 4096
    PR_TUNABLE_SCOREBOARD_BUFFER_SIZE = 80
    PR_TUNABLE_SCOREBOARD_SCRUB_TIMER = 30
    PR_TUNABLE_SELECT_TIMEOUT = 30
    PR_TUNABLE_TIMEOUTIDENT = 10
    PR_TUNABLE_TIMEOUTIDLE = 600
    PR_TUNABLE_TIMEOUTLINGER = 10
    PR_TUNABLE_TIMEOUTLOGIN = 300
    PR_TUNABLE_TIMEOUTNOXFER = 300
    PR_TUNABLE_TIMEOUTSTALLED = 3600
    PR_TUNABLE_XFER_SCOREBOARD_UPDATES = 10

proftpd.conf:

# Includes DSO modules
Include /etc/proftpd/modules.conf

# Set off to disable IPv6 support which is annoying on IPv4 only boxes.
UseIPv6 off
# If set on you can experience a longer connection delay in many cases.
<IfModule mod_ident.c>
  IdentLookups off
</IfModule>

ServerName "My FTP server"
# Read README.Debian for more information on proper configuration.
ServerType standalone
DeferWelcome off

DefaultServer on
ShowSymlinks on

TimeoutNoTransfer 600
TimeoutStalled 600
TimeoutIdle 1200

DisplayLogin welcome.msg
DisplayChdir .message true
ListOptions "-l"

DenyFilter \*.*/

<IfModule mod_dso.c>
  LoadModule mod_case.c
</IfModule>

# Use this to jail all users in their homes
DefaultRoot ~

# Port 21 is the standard FTP port.
Port 21

# In some cases you have to specify passive ports range to by-pass firewall limitations. Ephemeral ports can be used for that, but feel free to use a more narrow range.
PassivePorts 49152 65534

# To prevent DoS attacks, set the maximum number of child processes to 30.  If you need to allow more than 30 concurrent connections at once, simply increase this value.  Note that this ONLY works in standalone mode, in inetd mode you should use an inetd server that allows you to limit maximum number of processes per service (such as xinetd)
MaxInstances 30

# Set the user and group that the server normally runs at.
User proftpd
Group nogroup

# Umask 022 is a good standard umask to prevent new files and dirs (second parm) from being group and world writable.
Umask 022 022
# Normally, we want files to be overwriteable.
AllowOverwrite off

TransferLog /var/log/proftpd/xferlog
SystemLog /var/log/proftpd/proftpd.log
DebugLevel              10

# In order to keep log file dates consistent after chroot, use timezone info from /etc/localtime.  If this is not set, and proftpd is configured to chroot (e.g. DefaultRoot or <Anonymous>), it will use the non-daylight savings timezone regardless of whether DST is in effect.
#SetEnv TZ :/etc/localtime

<IfModule mod_quotatab.c>
QuotaEngine off
</IfModule>

<IfModule mod_ratio.c>
Ratios off
</IfModule>

# Delay engine reduces impact of the so-called Timing Attack described in http://www.securityfocus.com/bid/11430/discuss
# It is on by default.
<IfModule mod_delay.c>
DelayEngine on
</IfModule>

<IfModule mod_ctrls.c>
ControlsEngine off
ControlsMaxClients 2
ControlsLog /var/log/proftpd/controls.log
ControlsInterval 5
ControlsSocket /var/run/proftpd/proftpd.sock
</IfModule>

<IfModule mod_ctrls_admin.c>
AdminControlsEngine off
</IfModule>

Include /etc/proftpd/conf.d/
TraceLog                       /var/log/proftpd/trace.log
Trace                          DEFAULT:20

  <IfModule mod_rewrite.c>
    RewriteEngine on
    RewriteLog /var/log/proftpd/rewrite.log
    # Use the replaceall internal RewriteMap
    RewriteMap replace int:replaceall
    RewriteCondition %m RETR|SIZE|LIST|CWD
    RewriteRule (.*) "${replace:!$1!\\!/}"
  </IfModule>

  <IfModule mod_case.c>
     CaseEngine on
     CaseIgnore RETR,SIZE,LIST,CWD
  </IfModule>
@Castaglia Castaglia self-assigned this Aug 14, 2021
@Castaglia
Copy link
Member

Your newer ProFTPD version was built using the --enable-pcre configure option, which enables use of the PCRE library -- and PCRE expressions are indeed slightly different from POSIX regular expressions. I'll see if there's a way to achieve the same functionality using PCRE regexes.

@Castaglia
Copy link
Member

Hmm. There are some interesting discrepancies in your reported logs.

For example, from the ProFTPD 1.3.6c logs you provided:

<regexp:9>: executing POSIX regex '(.*)' against subject '/folder1\folder2'
<regexp:9>: executing PCRE regex 'RETR|SIZE|LIST|CWD' against subject 'CWD'

Why do we see both POSIX and PCRE regexes reported, if your ProFTPD 1.3.6c was not built with PCRE support?

And for the provided 1.3.7a log messages:

<regexp:9>: executing PCRE regex '(.*)' against subject '/folder1\folder2'
<regexp:9>: executing PCRE regex 'RETR|SIZE|LIST|CWD' against subject 'CWD'
<command:7>: dispatching PRE_CMD command 'CWD /folder1\folder2' to mod_core.c
<command:7>: dispatching PRE_CMD command 'CWD /folder1\folder2' to mod_core.c
<regexp:9>: executing PCRE regex '\*.*/' against subject '/folder1\folder2'
<regexp:9>: PCRE regex '\*.*/' failed to match subject '/folder1\folder2': subject did not match pattern

where is that \*.*/ regex coming from? Ah, from your DenyFilter setting:

DenyFilter \*.*/

Are you saying that your CWD is being rejected, failing? Or that the failure is not because of the DenyFilter, but because mod_rewrite is not rewriting that path properly?

@mdell-seradex
Copy link
Author

Hi @Castaglia.
Oh, interesting. I did not notice the DenyFilter.
I see now that the reason I missed that is because both the 1.3.6c server and the 1.3.7a server have DenyFilter in the Global settings.
An important difference between them is that the 1.3.6c server is using a VirtualHost, but the 1.3.7a server does not.
Also there is not DenyFilter in the 1.3.6c VirtualHost.

I tried pushing the settings from the 1.3.6c server to the 1.3.7a server, but the co-worker that created the new server created it very differently and I could not get it to work. I tried to modify them to get it to work, but to no avail. Every time I logged in and it tried to get the folder list, it would just hang.

I believed the issue was because mod_rewrite was not rewriting the path properly, but perhaps it is because of the DenyFilter?
The part that doesn't make sense is that if I specify the path using the correct slashes, it works as expected. I can get in to the FTP site and browse it with no issues.
I would have thought the the DenyFilter would be blocking it.

@mdell-seradex
Copy link
Author

I recall seeing some kind of conflicts when given an example for this kind of rewrite rule: RewriteRule (.*) "${replace:!$1!\\!/}"
Some examples showed it as this instead RewriteRule (.*) "${replace:!$1!\\\\!/}".
Could the difference be whether it is using POSIX or PCRE regexes?

@Castaglia
Copy link
Member

Castaglia commented Aug 14, 2021

The number of backslash (\) characters to use depends on the quoting. Both of the following should be equivalent:

RewriteRule (.*) ${replace:!$1!\\!/}
RewriteRule (.*) "${replace:!$1!\\\\!/}"

The reason for the extra backslashes in the second construction is that it is enclosed in double quotes, which add extra escaping requirements.

@Castaglia
Copy link
Member

However, the more I dig into this locally, the more evidence I find that this is a bug in ProFTPD's handling of matched PCRE (vs POSIX) regular expressions; it is not processing the matched groups properly.

In addition, I think it would be nice if ProFTPD had a way to say "I only want to use POSIX regexes, even if PCRE support is enabled" as well.

@mdell-seradex
Copy link
Author

mdell-seradex commented Aug 14, 2021

That is odd because in the VirtualHost on the 1.3.6c server it has this which works correctly: RewriteRule (.*) "${replace:!$1!\\!/}".
I read recently about different version of regex using backslashes ( \ ) differently. It says when BRE (Basic Regular Expressions) is used things like brackets need to be escaped to be treated as special characters, but when using ERE (Extended Regular Expressions) brackets, for example, need to be escaped to be treated as regular characters. There is also SRE (Simple Regular Expressions), but that has been deprecated. https://en.wikipedia.org/wiki/Regular_expression#Standards

The meaning of metacharacters escaped with a backslash is reversed for some characters in the POSIX Extended Regular Expression (ERE) syntax. With this syntax, a backslash causes the metacharacter to be treated as a literal character. So, for example, \( \) is now ( ) and \{ \} is now { }. Additionally, support is removed for \n backreferences and ...

Perhaps on the 1.3.7a server I need to use RewriteRule \(\.\*\) "${replace:!$1!\\\\!/}" or maybe RewriteRule \(.*\) "${replace:!$1!\\!/}", I am not really sure.

@mdell-seradex
Copy link
Author

FYI, if you look at #1282, you will see that on our 1.3.6c server, it is also built using the --enable-pcre configure option.

@Castaglia
Copy link
Member

Due to the bug where matched PCRE expressions do not have the matched/captured groups handled properly, it won't matter what number of backslashes are used, unfortunately. It's a code bug.

What's interesting, then, is that your 1.3.6c server, with --enable-pcre support, says:

<regexp:9>: executing POSIX regex '(.*)' against subject '/folder1\folder2'

which is probably why it is working as you expect, given that it is matching as a POSIX regular expression, not a PCRE expression.

@mdell-seradex
Copy link
Author

That is why I was wondering if perhaps PCRE is using BRE instead of ERE.
Unfortunately, I don't know what the affect of the many configuration options has on the compilation, so I do not know why v1.3.6c is using POSIX regex, but v1.3.7a is using PCRE regex.
I take it that there is no setting in the .conf files that could change that behaviour currently.

What I do find interesting is that this is in the rewrite logs, which indicates that maybe it is not capturing the data??:

rewrite_match_cond(): checking regex cond against 'CWD'
rewrite_fixup(): condition met
rewrite_fixup(): executing RewriteRule
rewrite_subst(): original pattern: '${replace:!$1!\!/}'
rewrite_subst_backrefs(): replacing backref '$1' with ''
rewrite_subst(): rule backref subst'd pattern: '${replace:!!\!/}'
rewrite_subst(): pattern '${replace:!$1!\!/}' had no cond backrefs
rewrite_subst(): var subst'd pattern: '${replace:!!\!/}'
rewrite_parse_map_str(): parsing '${replace:!!\!/}'
rewrite_subst_maps(): map name: 'replace'
rewrite_subst_maps(): lookup key: '!!\!/'
rewrite_subst_maps(): default value: ''
rewrite_subst_maps(): mapping '!!\!/' using 'replace'
rewrite_map_int_replaceall(): actual key: ''
rewrite_map_int_replaceall(): replacing '\' with '/'
rewrite_map_int_replaceall(): '\' does not occur in given key ''
rewrite_subst_maps(): internal map 'replace' returned ''
rewrite_subst_maps(): substituting '' for '${replace:!!\!/}'
rewrite_parse_map_str(): parsing ''
rewrite_subst(): maps subst'd pattern: ''
rewrite_subst(): env subst'd pattern: ''
rewrite_fixup(): error processing RewriteRule: generated empty command argument, which is not allowed

@mdell-seradex
Copy link
Author

FYI, I just tried RewriteRule \(.*\) "${replace:!$1!\\!/}" because I figured "why not?", but as you suspected/knew, it did not work.

@Castaglia Castaglia changed the title mod_rewrite not changing \ to / in v1.3.7a but works on other server in v1.3.6c PCRE expressions with capture groups are not being handled properly Aug 14, 2021
Castaglia added a commit that referenced this issue Aug 14, 2021
…sue1300

Issue #1300: Properly implement capturing groups for PCRE expressions.
Castaglia added a commit that referenced this issue Aug 14, 2021
@Castaglia
Copy link
Member

This should now be fixed in master; fix was backported to the 1.3.7 branch.

@mdell-seradex
Copy link
Author

Wow! Thank you.
With my setup, can I deploy this update somehow using prxs?
Is it just as simple as downloading the updated mod_rewrite.c and compiling via prxs?
It appears to be a shared module according to the configuration.

@mdell-seradex
Copy link
Author

mdell-seradex commented Aug 14, 2021

Looking at the commit more closely, it appears that the change is not specific to mod_rewrite.c.
It looks like your change is in the source for proftpd itself. As such it sounds like I would have to compile a whole new version, not something I was looking forward to do, considering how well it went last time.

@Castaglia
Copy link
Member

Unfortunately, the core issue/problem was in the core Regexp API in ProFTPD; the mod_rewrite module uses this API quite a bit, but it is not the only user of the API. So yes, this will unfortunately require compiling a new ProFTPD version, not just the mod_rewrite module.

Castaglia added a commit that referenced this issue Aug 14, 2021
…ndle

all regular expressions as POSIX, regardless of PCRE support.
Castaglia added a commit that referenced this issue Aug 14, 2021
…ndle

all regular expressions as POSIX, regardless of PCRE support.
Castaglia added a commit that referenced this issue Aug 14, 2021
As a follow-on to Issue #1300, make it possible to tell ProFTPD to ha…
@mdell-seradex
Copy link
Author

mdell-seradex commented Aug 14, 2021

I believe I have informed the package maintainers of this issue (hopefully correctly).
See https://bugs.launchpad.net/ubuntu/+source/proftpd-dfsg/+bug/1939964.
I referred them to here for additional information.

@mdell-seradex
Copy link
Author

mdell-seradex commented Aug 27, 2021

So apparently launchpad.net was the wrong place to log this for the package maintainers to be made aware.
I was informed that it is tracked here: https://tracker.debian.org/pkg/proftpd-dfsg since the Ubuntu package is typically made on Debian and ported to Ubuntu.
I am doing that now.

Apparently they can only be logged using the reportbug app or by email (boooo, hiss!), sigh.
See https://www.debian.org/Bugs/Reporting

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

No branches or pull requests

2 participants