jvinet / knock

A port-knocking daemon
http://www.zeroflux.org/projects/knock
GNU General Public License v2.0
549 stars 113 forks source link

knockd regression in commit dc6ba81 #77

Closed ehdis closed 3 years ago

ehdis commented 3 years ago

Since commit dc6ba81

./knockd -D -v -i eth0 
config: new section: 'options]'
config: usesyslog
config: new section: 'openSSH]'
config: openSSH]: sequence: 7000:tcp,8000:tcp,9000:tcp
config: openSSH]: seq_timeout: 10
config: tcp flag: SYN
config: openSSH]: start_command: /usr/sbin/iptables -A INPUT -s %IP% --dport 22 -j ACCEPT
error: section 'options]' has an empty knock sequence
usage: knockd [options]
options:
  -i, --interface <int>  network interface to listen on (default "eth0")
  -d, --daemon           run as a daemon
  -c, --config <file>    use an alternate config file
  -D, --debug            output debug messages
  -l, --lookup           lookup DNS names (may be a security risk)
  -p, --pidfile          use an alternate pidfile
  -g, --logfile          use an alternate logfile
  -v, --verbose          be verbose
  -V, --version          display version
  -h, --help             this help

Reverting to strlen helps: https://github.com/jvinet/knock/blob/master/src/knockd.c#L576

dimkr commented 3 years ago

strlen() is not the solution here. If strncpy() fails because the section is >= 256 characters, strlen() returns results bigger than 256 and assigning \0 to section+strlen(section) will zero some random byte on the stack.

dimkr commented 3 years ago

In fact, it looks like it should be sizeof(section)-2 if originally, this line had strlen(section)-1. The comparison to 'options" doesn't work because the section name is options[.

jvinet commented 3 years ago

Fixed via PR, thanks @dimkr