summaryrefslogtreecommitdiff
path: root/scp.c
diff options
context:
space:
mode:
authordjm@openbsd.org <djm@openbsd.org>2020-05-01 06:31:42 +0000
committerDamien Miller <djm@mindrot.org>2020-05-01 16:40:11 +1000
commitaad87b88fc2536b1ea023213729aaf4eaabe1894 (patch)
tree95cba7130275111af320e414a1451090a38b0db6 /scp.c
parent31909696c4620c431dd55f6cd15db65c4e9b98da (diff)
upstream: when receving a file in sink(), be careful to send at
most a single error response after the file has been opened. Otherwise the source() and sink() can become desyncronised. Reported by Daniel Goujot, Georges-Axel Jaloyan, Ryan Lahfa, and David Naccache. ok deraadt@ markus@ OpenBSD-Commit-ID: 6c14d233c97349cb811a8f7921ded3ae7d9e0035
Diffstat (limited to 'scp.c')
-rw-r--r--scp.c96
1 files changed, 59 insertions, 37 deletions
diff --git a/scp.c b/scp.c
index 812ab5301..439025980 100644
--- a/scp.c
+++ b/scp.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: scp.c,v 1.208 2020/04/30 17:07:10 markus Exp $ */ 1/* $OpenBSD: scp.c,v 1.209 2020/05/01 06:31:42 djm Exp $ */
2/* 2/*
3 * scp - secure remote copy. This is basically patched BSD rcp which 3 * scp - secure remote copy. This is basically patched BSD rcp which
4 * uses ssh to do the data transfer (instead of using rcmd). 4 * uses ssh to do the data transfer (instead of using rcmd).
@@ -374,6 +374,7 @@ BUF *allocbuf(BUF *, int, int);
374void lostconn(int); 374void lostconn(int);
375int okname(char *); 375int okname(char *);
376void run_err(const char *,...); 376void run_err(const char *,...);
377int note_err(const char *,...);
377void verifydir(char *); 378void verifydir(char *);
378 379
379struct passwd *pwd; 380struct passwd *pwd;
@@ -1231,9 +1232,6 @@ sink(int argc, char **argv, const char *src)
1231{ 1232{
1232 static BUF buffer; 1233 static BUF buffer;
1233 struct stat stb; 1234 struct stat stb;
1234 enum {
1235 YES, NO, DISPLAYED
1236 } wrerr;
1237 BUF *bp; 1235 BUF *bp;
1238 off_t i; 1236 off_t i;
1239 size_t j, count; 1237 size_t j, count;
@@ -1241,7 +1239,7 @@ sink(int argc, char **argv, const char *src)
1241 mode_t mode, omode, mask; 1239 mode_t mode, omode, mask;
1242 off_t size, statbytes; 1240 off_t size, statbytes;
1243 unsigned long long ull; 1241 unsigned long long ull;
1244 int setimes, targisdir, wrerrno = 0; 1242 int setimes, targisdir, wrerr;
1245 char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048]; 1243 char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048];
1246 char **patterns = NULL; 1244 char **patterns = NULL;
1247 size_t n, npatterns = 0; 1245 size_t n, npatterns = 0;
@@ -1450,8 +1448,13 @@ bad: run_err("%s: %s", np, strerror(errno));
1450 continue; 1448 continue;
1451 } 1449 }
1452 cp = bp->buf; 1450 cp = bp->buf;
1453 wrerr = NO; 1451 wrerr = 0;
1454 1452
1453 /*
1454 * NB. do not use run_err() unless immediately followed by
1455 * exit() below as it may send a spurious reply that might
1456 * desyncronise us from the peer. Use note_err() instead.
1457 */
1455 statbytes = 0; 1458 statbytes = 0;
1456 if (showprogress) 1459 if (showprogress)
1457 start_progress_meter(curfile, size, &statbytes); 1460 start_progress_meter(curfile, size, &statbytes);
@@ -1476,11 +1479,12 @@ bad: run_err("%s: %s", np, strerror(errno));
1476 1479
1477 if (count == bp->cnt) { 1480 if (count == bp->cnt) {
1478 /* Keep reading so we stay sync'd up. */ 1481 /* Keep reading so we stay sync'd up. */
1479 if (wrerr == NO) { 1482 if (!wrerr) {
1480 if (atomicio(vwrite, ofd, bp->buf, 1483 if (atomicio(vwrite, ofd, bp->buf,
1481 count) != count) { 1484 count) != count) {
1482 wrerr = YES; 1485 note_err("%s: %s", np,
1483 wrerrno = errno; 1486 strerror(errno));
1487 wrerr = 1;
1484 } 1488 }
1485 } 1489 }
1486 count = 0; 1490 count = 0;
@@ -1488,16 +1492,14 @@ bad: run_err("%s: %s", np, strerror(errno));
1488 } 1492 }
1489 } 1493 }
1490 unset_nonblock(remin); 1494 unset_nonblock(remin);
1491 if (count != 0 && wrerr == NO && 1495 if (count != 0 && !wrerr &&
1492 atomicio(vwrite, ofd, bp->buf, count) != count) { 1496 atomicio(vwrite, ofd, bp->buf, count) != count) {
1493 wrerr = YES; 1497 note_err("%s: %s", np, strerror(errno));
1494 wrerrno = errno; 1498 wrerr = 1;
1495 }
1496 if (wrerr == NO && (!exists || S_ISREG(stb.st_mode)) &&
1497 ftruncate(ofd, size) != 0) {
1498 run_err("%s: truncate: %s", np, strerror(errno));
1499 wrerr = DISPLAYED;
1500 } 1499 }
1500 if (!wrerr && (!exists || S_ISREG(stb.st_mode)) &&
1501 ftruncate(ofd, size) != 0)
1502 note_err("%s: truncate: %s", np, strerror(errno));
1501 if (pflag) { 1503 if (pflag) {
1502 if (exists || omode != mode) 1504 if (exists || omode != mode)
1503#ifdef HAVE_FCHMOD 1505#ifdef HAVE_FCHMOD
@@ -1505,9 +1507,8 @@ bad: run_err("%s: %s", np, strerror(errno));
1505#else /* HAVE_FCHMOD */ 1507#else /* HAVE_FCHMOD */
1506 if (chmod(np, omode)) { 1508 if (chmod(np, omode)) {
1507#endif /* HAVE_FCHMOD */ 1509#endif /* HAVE_FCHMOD */
1508 run_err("%s: set mode: %s", 1510 note_err("%s: set mode: %s",
1509 np, strerror(errno)); 1511 np, strerror(errno));
1510 wrerr = DISPLAYED;
1511 } 1512 }
1512 } else { 1513 } else {
1513 if (!exists && omode != mode) 1514 if (!exists && omode != mode)
@@ -1516,36 +1517,25 @@ bad: run_err("%s: %s", np, strerror(errno));
1516#else /* HAVE_FCHMOD */ 1517#else /* HAVE_FCHMOD */
1517 if (chmod(np, omode & ~mask)) { 1518 if (chmod(np, omode & ~mask)) {
1518#endif /* HAVE_FCHMOD */ 1519#endif /* HAVE_FCHMOD */
1519 run_err("%s: set mode: %s", 1520 note_err("%s: set mode: %s",
1520 np, strerror(errno)); 1521 np, strerror(errno));
1521 wrerr = DISPLAYED;
1522 } 1522 }
1523 } 1523 }
1524 if (close(ofd) == -1) { 1524 if (close(ofd) == -1)
1525 wrerr = YES; 1525 note_err(np, "%s: close: %s", np, strerror(errno));
1526 wrerrno = errno;
1527 }
1528 (void) response(); 1526 (void) response();
1529 if (showprogress) 1527 if (showprogress)
1530 stop_progress_meter(); 1528 stop_progress_meter();
1531 if (setimes && wrerr == NO) { 1529 if (setimes && !wrerr) {
1532 setimes = 0; 1530 setimes = 0;
1533 if (utimes(np, tv) == -1) { 1531 if (utimes(np, tv) == -1) {
1534 run_err("%s: set times: %s", 1532 note_err("%s: set times: %s",
1535 np, strerror(errno)); 1533 np, strerror(errno));
1536 wrerr = DISPLAYED;
1537 } 1534 }
1538 } 1535 }
1539 switch (wrerr) { 1536 /* If no error was noted then signal success for this file */
1540 case YES: 1537 if (note_err(NULL) == 0)
1541 run_err("%s: %s", np, strerror(wrerrno));
1542 break;
1543 case NO:
1544 (void) atomicio(vwrite, remout, "", 1); 1538 (void) atomicio(vwrite, remout, "", 1);
1545 break;
1546 case DISPLAYED:
1547 break;
1548 }
1549 } 1539 }
1550done: 1540done:
1551 for (n = 0; n < npatterns; n++) 1541 for (n = 0; n < npatterns; n++)
@@ -1633,6 +1623,38 @@ run_err(const char *fmt,...)
1633 } 1623 }
1634} 1624}
1635 1625
1626/*
1627 * Notes a sink error for sending at the end of a file transfer. Returns 0 if
1628 * no error has been noted or -1 otherwise. Use note_err(NULL) to flush
1629 * any active error at the end of the transfer.
1630 */
1631int
1632note_err(const char *fmt, ...)
1633{
1634 static char *emsg;
1635 va_list ap;
1636
1637 /* Replay any previously-noted error */
1638 if (fmt == NULL) {
1639 if (emsg == NULL)
1640 return 0;
1641 run_err("%s", emsg);
1642 free(emsg);
1643 emsg = NULL;
1644 return -1;
1645 }
1646
1647 errs++;
1648 /* Prefer first-noted error */
1649 if (emsg != NULL)
1650 return -1;
1651
1652 va_start(ap, fmt);
1653 vasnmprintf(&emsg, INT_MAX, NULL, fmt, ap);
1654 va_end(ap);
1655 return -1;
1656}
1657
1636void 1658void
1637verifydir(char *cp) 1659verifydir(char *cp)
1638{ 1660{