Bug 225996 - tftpd(8): tftpd doesn't abort on a WRQ access violation
Summary: tftpd(8): tftpd doesn't abort on a WRQ access violation
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL:
Keywords:
: 100914 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-02-18 03:38 UTC by Alan Somers
Modified: 2018-08-22 16:41 UTC (History)
2 users (show)

See Also:
asomers: mfc-stable11+
asomers: mfc-stable10-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer freebsd_triage 2018-02-18 03:38:53 UTC
On a WRQ (write request) tftpd checks whether the client has access permission for the file in question.  If not, then the write is prevented.  However, tftpd doesn't reply with an ERROR packet, nor does it abort.  Instead, it tries to receive the packet anyway.  The bug is in tftp_wrq() at line 543.  There is no error handling for ecode != 0.

The symptom is slightly different depending on the nature of the error.  If the target file is nonexistent and tftpd lacks permission to create it, then tftpd will willingly receive the file, but not write it anywhere.  If the file exists but is not writable, then tftpd will fail to ACK to WRQ.

tftp> put small.txt nonexistent
Try 1, didn't receive answer from remote.
Sent 7 bytes during 5.1 seconds in 1 blocks
tftp> put small.txt small.txt
Timeout #1 on ACK 1
Timeout #3 on ACK 1
Timeout #5 send ACK 1 giving up
Comment 1 commit-hook freebsd_committer freebsd_triage 2018-03-09 15:30:58 UTC
A commit references this bug:

Author: asomers
Date: Fri Mar  9 15:30:20 UTC 2018
New revision: 330696
URL: https://svnweb.freebsd.org/changeset/base/330696

Log:
  Add some functional tests for tftpd(8)

  tftpd(8) is difficult to test in isolation due to its relationship with
  inetd.  Create a test program that mimics the behavior of tftp(1) and
  inetd(8) and verifies tftpd's response in several different scenarios.

  These test cases cover all of the basic TFTP protocol, but not the optional
  parts.

  PR:		157700
  PR:		225996
  PR:		226004
  PR:		226005
  MFC after:	3 weeks
  Differential Revision:	https://reviews.freebsd.org/D14310

Changes:
  head/libexec/tftpd/Makefile
  head/libexec/tftpd/tests/
  head/libexec/tftpd/tests/Makefile
  head/libexec/tftpd/tests/functional.c
Comment 2 commit-hook freebsd_committer freebsd_triage 2018-03-10 01:44:57 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 01:43:56 UTC 2018
New revision: 330719
URL: https://svnweb.freebsd.org/changeset/base/330719

Log:
  tftpd: Abort on an WRQ access violation

  On a WRQ (write request) tftpd checks whether the client has access
  permission for the file in question.  If not, then the write is prevented.
  However, tftpd doesn't reply with an ERROR packet, nor does it abort.
  Instead, it tries to receive the packet anyway.

  The symptom is slightly different depending on the nature of the error.  If
  the target file is nonexistent and tftpd lacks permission to create it, then
  tftpd will willingly receive the file, but not write it anywhere.  If the
  file exists but is not writable, then tftpd will fail to ACK to WRQ.

  PR:		225996
  MFC after:	3 weeks

Changes:
  head/libexec/tftpd/tests/functional.c
  head/libexec/tftpd/tftpd.c
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-04-16 16:32:32 UTC
A commit references this bug:

Author: asomers
Date: Mon Apr 16 16:32:01 UTC 2018
New revision: 332608
URL: https://svnweb.freebsd.org/changeset/base/332608

Log:
  MFC r330696, r330709, r330742, r331358

  r330696:
  Add some functional tests for tftpd(8)

  tftpd(8) is difficult to test in isolation due to its relationship with
  inetd.  Create a test program that mimics the behavior of tftp(1) and
  inetd(8) and verifies tftpd's response in several different scenarios.

  These test cases cover all of the basic TFTP protocol, but not the optional
  parts.

  PR:		157700
  PR:		225996
  PR:		226004
  PR:		226005
  Differential Revision:	https://reviews.freebsd.org/D14310

  r330709:
  Commit missing file from r330696

  X-MFC-With:	330696

  r330742:
  tftpd: fix the build of tests on i386 after 330696

  It's those darn printf format specifiers again

  Reported by:	cy, kibab
  X-MFC-With:	330696

  r331358:
  tftpd: misc Coverity cleanup in the tests

  A bunch of unchecked return values from open(2) and read(2)

  Reported by:	Coverity
  CID:		1386900, 1386911, 1386926, 1386928, 1386932, 1386942
  CID:		1386961, 1386979
  X-MFC-With:	330696

Changes:
_U  stable/11/
  stable/11/etc/mtree/BSD.tests.dist
  stable/11/libexec/tftpd/Makefile
  stable/11/libexec/tftpd/tests/
  stable/11/libexec/tftpd/tests/functional.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2018-04-16 16:34:40 UTC
A commit references this bug:

Author: asomers
Date: Mon Apr 16 16:33:36 UTC 2018
New revision: 332609
URL: https://svnweb.freebsd.org/changeset/base/332609

Log:
  MFC r330710, r330718-r330720

  r330710:
  tftpd: Flush files as soon as they are fully received

  On an RRQ, tftpd doesn't exit as soon as it's finished receiving a file.
  Instead, it waits five seconds just in case the client didn't receive the
  server's last ACK and decides to resend the final DATA packet.
  Unfortunately, this created a 5 second delay from when the client thinks
  it's done sending the file, and when the file is available for other
  processes.

  Fix this bug by closing the file as soon as receipt is finished.

  PR:			157700
  Reported by:		Barry Mishler <barry_mishler@yahoo.com>

  r330718:
  tftpd: Verify world-writability for WRQ when using relative paths

  tftpd(8) says that files may only be written if they already exist and are
  publicly writable.  tftpd.c verifies that a file is publicly writable if it
  uses an absolute pathname.  However, if the pathname is relative, that check
  is skipped.  Fix it.

  Note that this is not a security vulnerability, because the transfer
  ultimately doesn't work unless the file already exists and is owned by user
  nobody.  Also, this bug does not affect the default configuration, because
  the default uses the "-s" option which makes all pathnames absolute.

  PR:		226004

  r330719:
  tftpd: Abort on an WRQ access violation

  On a WRQ (write request) tftpd checks whether the client has access
  permission for the file in question.  If not, then the write is prevented.
  However, tftpd doesn't reply with an ERROR packet, nor does it abort.
  Instead, it tries to receive the packet anyway.

  The symptom is slightly different depending on the nature of the error.  If
  the target file is nonexistent and tftpd lacks permission to create it, then
  tftpd will willingly receive the file, but not write it anywhere.  If the
  file exists but is not writable, then tftpd will fail to ACK to WRQ.

  PR:		225996

  r330720:
  tftpd: reject unknown opcodes

  If tftpd receives a command with an unknown opcode, it simply exits 1.  It
  doesn't send an ERROR packet, and the client will hang waiting for one.  Fix
  it.

  PR:		226005

Changes:
_U  stable/11/
  stable/11/libexec/tftpd/tests/functional.c
  stable/11/libexec/tftpd/tftp-transfer.c
  stable/11/libexec/tftpd/tftpd.c
  stable/11/usr.bin/tftp/tftp.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-08-03 14:14:14 UTC
A commit references this bug:

Author: asomers
Date: Fri Aug  3 14:13:16 UTC 2018
New revision: 337246
URL: https://svnweb.freebsd.org/changeset/base/337246

Log:
  MFC r330696, r330709, r330742, r331358

  r330696:
  Add some functional tests for tftpd(8)

  tftpd(8) is difficult to test in isolation due to its relationship with
  inetd.  Create a test program that mimics the behavior of tftp(1) and
  inetd(8) and verifies tftpd's response in several different scenarios.

  These test cases cover all of the basic TFTP protocol, but not the optional
  parts.

  PR:		157700
  PR:		225996
  PR:		226004
  PR:		226005
  Differential Revision:	https://reviews.freebsd.org/D14310

  r330709:
  Commit missing file from r330696

  X-MFC-With:	330696

  r330742:
  tftpd: fix the build of tests on i386 after 330696

  It's those darn printf format specifiers again

  Reported by:	cy, kibab
  X-MFC-With:	330696

  r331358:
  tftpd: misc Coverity cleanup in the tests

  A bunch of unchecked return values from open(2) and read(2)

  Reported by:	Coverity
  CID:		1386900, 1386911, 1386926, 1386928, 1386932, 1386942
  CID:		1386961, 1386979
  X-MFC-With:	330696

Changes:
_U  stable/10/
  stable/10/etc/mtree/BSD.tests.dist
  stable/10/libexec/tftpd/Makefile
  stable/10/libexec/tftpd/tests/
  stable/10/libexec/tftpd/tests/functional.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-08-03 14:19:28 UTC
A commit references this bug:

Author: asomers
Date: Fri Aug  3 14:19:10 UTC 2018
New revision: 337249
URL: https://svnweb.freebsd.org/changeset/base/337249

Log:
  MFC r330719:

  tftpd: Abort on an WRQ access violation

  On a WRQ (write request) tftpd checks whether the client has access
  permission for the file in question.  If not, then the write is prevented.
  However, tftpd doesn't reply with an ERROR packet, nor does it abort.
  Instead, it tries to receive the packet anyway.

  The symptom is slightly different depending on the nature of the error.  If
  the target file is nonexistent and tftpd lacks permission to create it, then
  tftpd will willingly receive the file, but not write it anywhere.  If the
  file exists but is not writable, then tftpd will fail to ACK to WRQ.

  PR:		225996

Changes:
_U  stable/10/
  stable/10/libexec/tftpd/tests/functional.c
  stable/10/libexec/tftpd/tftpd.c
Comment 7 Alan Somers freebsd_committer freebsd_triage 2018-08-22 16:41:06 UTC
*** Bug 100914 has been marked as a duplicate of this bug. ***