Opened 9 months ago

Closed 5 months ago

#19965 closed defect (fixed)

Log torrc option can't handle tab after severity

Reported by: arma Owned by: jryans
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: 0.2.1.10-alpha
Severity: Normal Keywords: regression
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Set your torrc file to be

log debug       file /tmp/debug.log

where that long space is a tab.

Then tor -f /path/to/torrc
and you'll get

Aug 23 15:23:49.300 [warn] Couldn't parse log levels in Log option 'Log debug        file /tmp/debug.log'
Aug 23 15:23:49.300 [warn] Failed to parse/validate config: Failed to validate Log options. See logs for details.
Aug 23 15:23:49.300 [err] Reading config failed--see warnings above.

It looks like the issue is in parse_log_severity_config() where we do stuff like

    space = strchr(cfg, ' ');

Reported by toralf.

Child Tickets

Change History (8)

comment:1 Changed 9 months ago by arma

I think technically this is a regression, since Tor 0.2.0.32 can handle this situation just fine.

Added in f56ba5f3d which went into 0.2.1.1-alpha.

comment:2 Changed 6 months ago by teor

  • Milestone changed from Tor: 0.2.??? to Tor: 0.3.???

Milestone renamed

comment:3 Changed 6 months ago by jryans

  • Owner set to jryans
  • Status changed from new to assigned

comment:4 Changed 6 months ago by jryans

  • Status changed from assigned to needs_review

I adjusted the parsing in parse_log_severity_config to accept any whitespace after the severity and added a basic unit test for this.

https://github.com/jryans/tor/commits/log-severity

comment:5 Changed 6 months ago by teor

  • Keywords regression added
  • Milestone changed from Tor: 0.3.??? to Tor: 0.3.0.x-final
  • Status changed from needs_review to merge_ready
  • Version set to Tor: 0.2.1.10-alpha

Seems good to me.
(I checked, and we don't do this anywhere else in option parsing.)

comment:6 Changed 6 months ago by cypherpunks

The NULL check at test_config.c:4917 is unnecessary because tor_free can handle NULLs.

comment:7 Changed 6 months ago by jryans

Thanks for pointing that out cypherpunks! Updated patch to remove redundant null check in test_config.c

comment:8 Changed 5 months ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

merged; thanks!

Note: See TracTickets for help on using tickets.