The async-signal-safe fix used internally FD 3 and FD 4 which could conflict with options fdin,fdout

This commit is contained in:
Gerhard Rieger 2018-07-29 11:12:30 +02:00
parent a3c688210f
commit 9f5abda361
5 changed files with 86 additions and 1 deletions

View file

@ -16,6 +16,11 @@ corrections:
compiling with MUSL libc failed. compiling with MUSL libc failed.
Problem reported by Kennedy33. Problem reported by Kennedy33.
The async signal safe diagnostic system used FDs 3 and 4 internally, so
use of appropriate fdin or fdout led to failures.
Test: DIAG_FDIN
Problem reported by Onur Sentürk.
testing: testing:
test.sh: Show a warning when phase-1 (insecure phase) of a security test.sh: Show a warning when phase-1 (insecure phase) of a security
test fails test fails

13
error.c
View file

@ -210,6 +210,19 @@ const char *diag_get_string(char what) {
return NULL; return NULL;
} }
/* make sure that the diag_sock fds do not have this num */
int diag_reserve_fd(int fd) {
DIAG_INIT;
if (diag_sock_send == fd) {
diag_sock_send = Dup(fd);
Close(fd);
}
if (diag_sock_recv == fd) {
diag_sock_recv = Dup(fd);
Close(fd);
}
return 0;
}
/* Linux and AIX syslog format: /* Linux and AIX syslog format:
Oct 4 17:10:37 hostname socat[52798]: D signal(13, 1) Oct 4 17:10:37 hostname socat[52798]: D signal(13, 1)

View file

@ -236,6 +236,7 @@ extern void diag_set(char what, const char *arg);
extern void diag_set_int(char what, int arg); extern void diag_set_int(char what, int arg);
extern int diag_get_int(char what); extern int diag_get_int(char what);
extern const char *diag_get_string(char what); extern const char *diag_get_string(char what);
extern int diag_reserve_fd(int fd);
extern int diag_dup(void); extern int diag_dup(void);
extern int diag_dup2(int newfd); extern int diag_dup2(int newfd);
extern void msg(int level, const char *format, ...); extern void msg(int level, const char *format, ...);

42
test.sh
View file

@ -12799,6 +12799,48 @@ esac
PORT=$((PORT+1)) PORT=$((PORT+1))
N=$((N+1)) N=$((N+1))
# The fix to "Make code async-signal-safe" used internally FD 3 and FD 4.
# Using option fdin=3 did not pass data to executed program.
NAME=DIAG_FDIN
case "$TESTS" in
*%$N%*|*%functions%*|*%bugs%*|*%exec%*|*%$NAME%*)
TEST="$NAME: test use of fdin=3"
# Use FD 3 explicitely with fdin and test if Socat passes data to executed
# program
if ! eval $NUMCOND; then :; else
tf="$td/test$N.stdout"
te="$td/test$N.stderr"
tdiff="$td/test$N.diff"
da="test$N $(date) $RANDOM"
CMD0="$TRACE $SOCAT $opts - SYSTEM:\"cat >&3 <&4\",fdin=4,fdout=3"
printf "test $F_n $TEST... " $N
echo "$da" |$TRACE $SOCAT $opts - SYSTEM:"cat <&3 >&4",fdin=3,fdout=4 >${tf}0 2>"${te}0"
rc0=$?
if [ $rc0 -ne 0 ]; then
$PRINTF "$FAILED\n"
echo "$CMD0"
cat "${te}0"
numFAIL=$((numFAIL+1))
listFAIL="$listFAIL $N"
elif echo "$da" |diff - ${tf}0 >$tdiff; then
$PRINTF "$OK\n"
numOK=$((numOK+1))
else
$PRINTF "$FAILED\n"
echo "$CMD0"
cat "${te}0"
cat "$tdiff"
numFAIL=$((numFAIL+1))
listFAIL="$listFAIL $N"
fi
fi # NUMCOND
;;
esac
PORT=$((PORT+1))
N=$((N+1))
################################################################################## ##################################################################################
#================================================================================= #=================================================================================
# here come tests that might affect your systems integrity. Put normal tests # here come tests that might affect your systems integrity. Put normal tests

View file

@ -168,7 +168,7 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
} }
#endif #endif
/* remember: fdin is the fs where the sub program reads from, thus it is /* remember: fdin is the fd where the sub program reads from, thus it is
sock0[]'s read fd */ sock0[]'s read fd */
/*! problem: when fdi==WRFD(sock[0]) or fdo==RDFD(sock[0]) */ /*! problem: when fdi==WRFD(sock[0]) or fdo==RDFD(sock[0]) */
if (rw != XIO_WRONLY) { if (rw != XIO_WRONLY) {
@ -177,6 +177,9 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
Warn2("fcntl(%d, F_SETFD, 0): %s", fdo, strerror(errno)); Warn2("fcntl(%d, F_SETFD, 0): %s", fdo, strerror(errno));
} }
} else { } else {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdo);
if (Dup2(XIO_GETWRFD(sock[0]), fdo) < 0) { if (Dup2(XIO_GETWRFD(sock[0]), fdo) < 0) {
Error3("dup2(%d, %d): %s", Error3("dup2(%d, %d): %s",
XIO_GETWRFD(sock[0]), fdo, strerror(errno)); XIO_GETWRFD(sock[0]), fdo, strerror(errno));
@ -190,6 +193,9 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
Warn2("fcntl(%d, F_SETFD, 0): %s", fdi, strerror(errno)); Warn2("fcntl(%d, F_SETFD, 0): %s", fdi, strerror(errno));
} }
} else { } else {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdi);
if (Dup2(XIO_GETRDFD(sock[0]), fdi) < 0) { if (Dup2(XIO_GETRDFD(sock[0]), fdi) < 0) {
Error3("dup2(%d, %d): %s)", Error3("dup2(%d, %d): %s)",
XIO_GETRDFD(sock[0]), fdi, strerror(errno)); XIO_GETRDFD(sock[0]), fdi, strerror(errno));
@ -425,12 +431,18 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
if (usepty) { if (usepty) {
Close(ptyfd); Close(ptyfd);
if (rw != XIO_RDONLY && fdi != ttyfd) { if (rw != XIO_RDONLY && fdi != ttyfd) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdi);
if (Dup2(ttyfd, fdi) < 0) { if (Dup2(ttyfd, fdi) < 0) {
Error3("dup2(%d, %d): %s", ttyfd, fdi, strerror(errno)); Error3("dup2(%d, %d): %s", ttyfd, fdi, strerror(errno));
return -1; } return -1; }
/*0 Info2("dup2(%d, %d)", ttyfd, fdi);*/ /*0 Info2("dup2(%d, %d)", ttyfd, fdi);*/
} }
if (rw != XIO_WRONLY && fdo != ttyfd) { if (rw != XIO_WRONLY && fdo != ttyfd) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdo);
if (Dup2(ttyfd, fdo) < 0) { if (Dup2(ttyfd, fdo) < 0) {
Error3("dup2(%d, %d): %s", ttyfd, fdo, strerror(errno)); Error3("dup2(%d, %d): %s", ttyfd, fdo, strerror(errno));
return -1; } return -1; }
@ -472,6 +484,9 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
} }
if (rw != XIO_WRONLY && rdpip[1] != fdo) { if (rw != XIO_WRONLY && rdpip[1] != fdo) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdo);
if (Dup2(rdpip[1], fdo) < 0) { if (Dup2(rdpip[1], fdo) < 0) {
Error3("dup2(%d, %d): %s", rdpip[1], fdo, strerror(errno)); Error3("dup2(%d, %d): %s", rdpip[1], fdo, strerror(errno));
return -1; return -1;
@ -481,6 +496,9 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
/*0 applyopts_cloexec(fdo, *copts);*/ /*0 applyopts_cloexec(fdo, *copts);*/
} }
if (rw != XIO_RDONLY && wrpip[0] != fdi) { if (rw != XIO_RDONLY && wrpip[0] != fdi) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdi);
if (Dup2(wrpip[0], fdi) < 0) { if (Dup2(wrpip[0], fdi) < 0) {
Error3("dup2(%d, %d): %s", wrpip[0], fdi, strerror(errno)); Error3("dup2(%d, %d): %s", wrpip[0], fdi, strerror(errno));
return -1; return -1;
@ -499,12 +517,18 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */
} else { /* socketpair */ } else { /* socketpair */
Close(sv[0]); Close(sv[0]);
if (rw != XIO_RDONLY && fdi != sv[1]) { if (rw != XIO_RDONLY && fdi != sv[1]) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdi);
if (Dup2(sv[1], fdi) < 0) { if (Dup2(sv[1], fdi) < 0) {
Error3("dup2(%d, %d): %s", sv[1], fdi, strerror(errno)); Error3("dup2(%d, %d): %s", sv[1], fdi, strerror(errno));
return -1; } return -1; }
/*0 Info2("dup2(%d, %d)", sv[1], fdi);*/ /*0 Info2("dup2(%d, %d)", sv[1], fdi);*/
} }
if (rw != XIO_WRONLY && fdo != sv[1]) { if (rw != XIO_WRONLY && fdo != sv[1]) {
/* make sure that the internal diagnostic socket pair fds do not conflict
with our choices */
diag_reserve_fd(fdo);
if (Dup2(sv[1], fdo) < 0) { if (Dup2(sv[1], fdo) < 0) {
Error3("dup2(%d, %d): %s", sv[1], fdo, strerror(errno)); Error3("dup2(%d, %d): %s", sv[1], fdo, strerror(errno));
return -1; } return -1; }