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

Cannot add regex containing comma in v6 #2834

Closed
patrickli opened this issue Nov 15, 2023 · 3 comments · Fixed by #2835
Closed

Cannot add regex containing comma in v6 #2834

patrickli opened this issue Nov 15, 2023 · 3 comments · Fixed by #2835

Comments

@patrickli
Copy link

Versions

  • Pi-hole: development-v6 af9b8d
  • AdminLTE: development-v6 e0ca2b2
  • FTL: development-v6 vDev-6e25f5c

Platform

  • OS and version: Alpine
  • Platform: Docker

Expected behavior

Trying to add a regex deny string ^www\.google\.[[:alpha:]]{2,3}(\.[[:alpha:]]{2})?$;reply=216.239.38.120.
It should be added successfully.

Actual behavior / bug

The regex is break into 2 items. It seems FTL is splitting the string by comma. This is not a good idea as comma means something in regex. I can add it successfully in v5.17.2.

A couple of times this also caused a segfault.

Docker log:

2023-11-15 16:33:46.596 [159/T168] WARNING: API: Regex validation failed (Missing '}')
2023-11-15 16:33:46.596 [159/T168] INFO: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
2023-11-15 16:33:46.596 [159/T168] INFO: ---------------------------->  FTL crashed!  <----------------------------
2023-11-15 16:33:46.596 [159/T168] INFO: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
2023-11-15 16:33:46.596 [159/T168] INFO: Please report a bug at https://github.com/pi-hole/FTL/issues
2023-11-15 16:33:46.596 [159/T168] INFO: and include in your report already the following details:
2023-11-15 16:33:46.596 [159/T168] INFO: FTL has been running for 55 seconds
2023-11-15 16:33:46.596 [159/T168] INFO: FTL branch: development-v6
2023-11-15 16:33:46.596 [159/T168] INFO: FTL version: vDev-6e25f5c
2023-11-15 16:33:46.597 [159/T168] INFO: FTL commit: 6e25f5ce
2023-11-15 16:33:46.597 [159/T168] INFO: FTL date: 2023-11-13 22:53:43 +0100
2023-11-15 16:33:46.597 [159/T168] INFO: FTL user: started as pihole, ended as pihole
2023-11-15 16:33:46.597 [159/T168] INFO: Compiled for linux/amd64 (compiled on CI) using cc (Alpine 12.2.1_git20220924-r10) 12.2.1 20220924
2023-11-15 16:33:46.597 [159/T168] INFO: Process details: MID: 159
2023-11-15 16:33:46.597 [159/T168] INFO:                  PID: 159
2023-11-15 16:33:46.597 [159/T168] INFO:                  TID: 168
2023-11-15 16:33:46.597 [159/T168] INFO:                  Name: civetweb-worker
2023-11-15 16:33:46.597 [159/T168] INFO: Received signal: Segmentation fault
2023-11-15 16:33:46.597 [159/T168] INFO:      at address: 0x10
2023-11-15 16:33:46.597 [159/T168] INFO:      with code:  SEGV_MAPERR (Address not mapped to object)
2023-11-15 16:33:46.597 [159/T168] INFO: !!! INFO: pihole-FTL has not been compiled with glibc/backtrace support, not generating one !!!
2023-11-15 16:33:46.597 [159/T168] INFO: ------ Listing content of directory /dev/shm ------
2023-11-15 16:33:46.597 [159/T168] INFO: File Mode User:Group      Size  Filename
2023-11-15 16:33:46.597 [159/T168] INFO: rwxrwxrwx root:root       280  .
2023-11-15 16:33:46.597 [159/T168] INFO: rwxr-xr-x root:root       340  ..
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole   544K  FTL-fifo-log
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole     4K  FTL-per-client-regex
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole    12K  FTL-dns-cache
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole     8K  FTL-overTime
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole   262K  FTL-queries
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole    20K  FTL-upstreams
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole    86K  FTL-clients
2023-11-15 16:33:46.597 [159/T168] INFO: rw------- pihole:pihole    12K  FTL-domains
2023-11-15 16:33:46.598 [159/T168] INFO: rw------- pihole:pihole   292  FTL-counters
2023-11-15 16:33:46.598 [159/T168] INFO: rw------- pihole:pihole    88  FTL-lock
2023-11-15 16:33:46.598 [159/T168] INFO: ---------------------------------------------------
2023-11-15 16:33:46.598 [159/T168] INFO: Please also include some lines from above the !!!!!!!!! header.
2023-11-15 16:33:46.598 [159/T168] INFO: Thank you for helping us to improve our FTL engine!
2023-11-15 16:33:46.598 [159/T168] INFO: Waiting for threads to join
2023-11-15 16:33:46.598 [159/T168] INFO: Thread database (0) is idle, terminating it.
2023-11-15 16:33:46.598 [159/T168] INFO: Thread housekeeper (1) is idle, terminating it.
2023-11-15 16:33:46.598 [159/T168] INFO: Thread DNS client (2) is idle, terminating it.
2023-11-15 16:33:46.598 [159/T168] INFO: All threads joined
2023-11-15 16:33:46.884 [159/T168] INFO: Stored 1/16 API sessions in the database

Steps to reproduce

Steps to reproduce the behavior:

  1. Go to 'Domains'
  2. Click on 'RegEx Filter'
  3. Enter the regex mentioned above and add.
  4. See error
@yubiuser
Copy link
Member

Thanks for the report, this is a regression of #2707

@DL6ER
Copy link
Member

DL6ER commented Nov 15, 2023

Indeed, we see two bugs here:

  1. comma should not be allowed for domains as it is meaningful in regex, and
  2. FTL should not unconditionally free memory it may not have allocated when reporting an error.

I will set up bugfixes for both and report back here.

@DL6ER
Copy link
Member

DL6ER commented Nov 15, 2023

I transferred this issue from FTL into the web realm as the issue itself is happening here. There are two bug fixes fixing the two individual issues above on their own:

  1. Do not accept comma to separate regex #2835
  2. Prevent unconditional free on error reporting FTL#1759

@yubiuser yubiuser linked a pull request Nov 16, 2023 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants