Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 16304a0

Browse files
nmischnmisch
committed
Add error-throwing wrappers for the printf family of functions.
All known standard library implementations of these functions can fail with ENOMEM. A caller neglecting to check for failure would experience missing output, information exposure, or a crash. Check return values within wrappers and code, currently just snprintf.c, that bypasses the wrappers. The wrappers do not return after an error, so their callers need not check. Back-patch to 9.0 (all supported versions). Popular free software standard library implementations do take pains to bypass malloc() in simple cases, but they risk ENOMEM for floating point numbers, positional arguments, large field widths, and large precisions. No specification demands such caution, so this commit regards every call to a printf family function as a potential threat. Injecting the wrappers implicitly is a compromise between patch scope and design goals. I would prefer to edit each call site to name a wrapper explicitly. libpq and the ECPG libraries would, ideally, convey errors to the caller rather than abort(). All that would be painfully invasive for a back-patched security fix, hence this compromise. Security: CVE-2015-3166
1 parent cac18a7 commit 16304a0

File tree

16 files changed

+300
-93
lines changed

16 files changed

+300
-93
lines changed

‎src/include/port.h

+53-27
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,11 @@ extern unsigned char pg_tolower(unsigned char ch);
126126
extern unsigned char pg_ascii_toupper(unsigned char ch);
127127
extern unsigned char pg_ascii_tolower(unsigned char ch);
128128

129-
#ifdef USE_REPL_SNPRINTF
130-
131129
/*
132-
* Versions of libintl >= 0.13 try to replace printf() and friends with
133-
* macros to their own versions that understand the %$ format. We do the
134-
* same, so disable their macros, if they exist.
130+
* Capture macro-compatible calls to printf() and friends, and redirect them
131+
* to wrappers that throw errors in lieu of reporting failure in a return
132+
* value. Versions of libintl >= 0.13 similarly redirect to versions that
133+
* understand the %$ format, so disable libintl macros first.
135134
*/
136135
#ifdef vsnprintf
137136
#undef vsnprintf
@@ -155,36 +154,63 @@ extern unsigned char pg_ascii_tolower(unsigned char ch);
155154
#undef printf
156155
#endif
157156

158-
extern int pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args);
159-
extern int pg_snprintf(char *str, size_t count, const char *fmt,...) pg_attribute_printf(3, 4);
160-
extern int pg_vsprintf(char *str, const char *fmt, va_list args);
161-
extern int pg_sprintf(char *str, const char *fmt,...) pg_attribute_printf(2, 3);
162-
extern int pg_vfprintf(FILE *stream, const char *fmt, va_list args);
163-
extern int pg_fprintf(FILE *stream, const char *fmt,...) pg_attribute_printf(2, 3);
164-
extern int pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);
157+
extern int
158+
vsnprintf_throw_on_fail(char *str, size_t count, const char *fmt, va_list args)
159+
pg_attribute_printf(3, 0);
160+
extern int
161+
snprintf_throw_on_fail(char *str, size_t count, const char *fmt,...)
162+
pg_attribute_printf(3, 4);
163+
extern int
164+
vsprintf_throw_on_fail(char *str, const char *fmt, va_list args)
165+
pg_attribute_printf(2, 0);
166+
extern int
167+
sprintf_throw_on_fail(char *str, const char *fmt,...)
168+
pg_attribute_printf(2, 3);
169+
extern int
170+
vfprintf_throw_on_fail(FILE *stream, const char *fmt, va_list args)
171+
pg_attribute_printf(2, 0);
172+
extern int
173+
fprintf_throw_on_fail(FILE *stream, const char *fmt,...)
174+
pg_attribute_printf(2, 3);
175+
extern int
176+
printf_throw_on_fail(const char *fmt,...)
177+
pg_attribute_printf(1, 2);
165178

166179
/*
167180
* The GCC-specific code below prevents the pg_attribute_printf above from
168181
* being replaced, and this is required because gcc doesn't know anything
169-
* about pg_printf.
182+
* about printf_throw_on_fail.
170183
*/
171184
#ifdef __GNUC__
172-
#define vsnprintf(...) pg_vsnprintf(__VA_ARGS__)
173-
#define snprintf(...) pg_snprintf(__VA_ARGS__)
174-
#define vsprintf(...) pg_vsprintf(__VA_ARGS__)
175-
#define sprintf(...) pg_sprintf(__VA_ARGS__)
176-
#define vfprintf(...) pg_vfprintf(__VA_ARGS__)
177-
#define fprintf(...) pg_fprintf(__VA_ARGS__)
178-
#define printf(...) pg_printf(__VA_ARGS__)
185+
#define vsnprintf(...) vsnprintf_throw_on_fail(__VA_ARGS__)
186+
#define snprintf(...) snprintf_throw_on_fail(__VA_ARGS__)
187+
#define vsprintf(...) vsprintf_throw_on_fail(__VA_ARGS__)
188+
#define sprintf(...) sprintf_throw_on_fail(__VA_ARGS__)
189+
#define vfprintf(...) vfprintf_throw_on_fail(__VA_ARGS__)
190+
#define fprintf(...) fprintf_throw_on_fail(__VA_ARGS__)
191+
#define printf(...) printf_throw_on_fail(__VA_ARGS__)
179192
#else
180-
#define vsnprintf pg_vsnprintf
181-
#define snprintf pg_snprintf
182-
#define vsprintf pg_vsprintf
183-
#define sprintf pg_sprintf
184-
#define vfprintf pg_vfprintf
185-
#define fprintf pg_fprintf
186-
#define printf pg_printf
193+
#define vsnprintf vsnprintf_throw_on_fail
194+
#define snprintf snprintf_throw_on_fail
195+
#define vsprintf vsprintf_throw_on_fail
196+
#define sprintf sprintf_throw_on_fail
197+
#define vfprintf vfprintf_throw_on_fail
198+
#define fprintf fprintf_throw_on_fail
199+
#define printf printf_throw_on_fail
187200
#endif
201+
202+
#ifdef USE_REPL_SNPRINTF
203+
204+
/* Code outside syswrap.c should not call these. */
205+
206+
extern int pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args);
207+
extern int pg_snprintf(char *str, size_t count, const char *fmt,...) pg_attribute_printf(3, 4);
208+
extern int pg_vsprintf(char *str, const char *fmt, va_list args);
209+
extern int pg_sprintf(char *str, const char *fmt,...) pg_attribute_printf(2, 3);
210+
extern int pg_vfprintf(FILE *stream, const char *fmt, va_list args);
211+
extern int pg_fprintf(FILE *stream, const char *fmt,...) pg_attribute_printf(2, 3);
212+
extern int pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);
213+
188214
#endif /* USE_REPL_SNPRINTF */
189215

190216
#if defined(WIN32)

‎src/interfaces/ecpg/compatlib/Makefile

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ submake-pgtypeslib:
4848
# Shared library stuff
4949
include $(top_srcdir)/src/Makefile.shlib
5050

51+
# XXX This library uses no symbols from snprintf.c.
5152
snprintf.c: % : $(top_srcdir)/src/port/%
5253
rm -f $@ && $(LN_S) $< .
5354

‎src/interfaces/ecpg/ecpglib/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
/pgstrcasecmp.c
66
/snprintf.c
77
/strlcpy.c
8+
/syswrap.c
89
/thread.c
910
/win32setlocale.c
1011
/isinf.c

‎src/interfaces/ecpg/ecpglib/Makefile

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ override CFLAGS += $(PTHREAD_CFLAGS)
2626
LIBS := $(filter-out -lpgport, $(LIBS))
2727

2828
OBJS= execute.o typename.o descriptor.o sqlda.o data.o error.o prepare.o memory.o \
29-
connect.o misc.o path.o pgstrcasecmp.o \
29+
connect.o misc.o path.o pgstrcasecmp.o syswrap.o \
3030
$(filter snprintf.o strlcpy.o win32setlocale.o isinf.o, $(LIBOBJS)) $(WIN32RES)
3131

3232
# thread.c is needed only for non-WIN32 implementation of path.c
@@ -55,7 +55,7 @@ include $(top_srcdir)/src/Makefile.shlib
5555
# necessarily use the same object files as the backend uses. Instead,
5656
# symlink the source files in here and build our own object file.
5757

58-
path.c pgstrcasecmp.c snprintf.c strlcpy.c thread.c win32setlocale.c isinf.c: % : $(top_srcdir)/src/port/%
58+
path.c pgstrcasecmp.c snprintf.c strlcpy.c syswrap.c thread.c win32setlocale.c isinf.c: % : $(top_srcdir)/src/port/%
5959
rm -f $@ && $(LN_S) $< .
6060

6161
misc.o: misc.c $(top_builddir)/src/port/pg_config_paths.h
@@ -72,6 +72,6 @@ uninstall: uninstall-lib
7272

7373
clean distclean: clean-lib
7474
rm -f $(OBJS)
75-
rm -f path.c pgstrcasecmp.c snprintf.c strlcpy.c thread.c win32setlocale.c isinf.c
75+
rm -f path.c pgstrcasecmp.c snprintf.c strlcpy.c syswrap.c thread.c win32setlocale.c isinf.c
7676

7777
maintainer-clean: distclean maintainer-clean-lib

‎src/interfaces/ecpg/pgtypeslib/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44
/pgstrcasecmp.c
55
/rint.c
66
/snprintf.c
7+
/syswrap.c

‎src/interfaces/ecpg/pgtypeslib/Makefile

+3-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ SHLIB_LINK += -lm
3030
SHLIB_EXPORTS = exports.txt
3131

3232
OBJS= numeric.o datetime.o common.o dt_common.o timestamp.o interval.o \
33-
pgstrcasecmp.o \
33+
pgstrcasecmp.o syswrap.o \
3434
$(filter rint.o snprintf.o, $(LIBOBJS)) $(WIN32RES)
3535

3636
all: all-lib
@@ -43,7 +43,7 @@ include $(top_srcdir)/src/Makefile.shlib
4343
# necessarily use the same object files as the backend uses. Instead,
4444
# symlink the source files in here and build our own object file.
4545

46-
pgstrcasecmp.c rint.c snprintf.c: % : $(top_srcdir)/src/port/%
46+
pgstrcasecmp.c rint.c snprintf.c syswrap.c: % : $(top_srcdir)/src/port/%
4747
rm -f $@ && $(LN_S) $< .
4848

4949
install: all installdirs install-lib
@@ -53,6 +53,6 @@ installdirs: installdirs-lib
5353
uninstall: uninstall-lib
5454

5555
clean distclean: clean-lib
56-
rm -f $(OBJS) pgstrcasecmp.c rint.c snprintf.c
56+
rm -f $(OBJS) pgstrcasecmp.c rint.c snprintf.c syswrap.c
5757

5858
maintainer-clean: distclean maintainer-clean-lib

‎src/interfaces/libpq/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
/strerror.c
1414
/strlcpy.c
1515
/system.c
16+
/syswrap.c
1617
/thread.c
1718
/win32error.c
1819
/win32setlocale.c

‎src/interfaces/libpq/Makefile

+3-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
3636
libpq-events.o
3737
# libpgport C files we always use
3838
OBJS += chklocale.o inet_net_ntop.o noblock.o pgstrcasecmp.o pqsignal.o \
39-
thread.o
39+
syswrap.o thread.o
4040
# libpgport C files that are needed if identified by configure
4141
OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
4242
# backend/libpq
@@ -93,7 +93,7 @@ backend_src = $(top_srcdir)/src/backend
9393
# For some libpgport modules, this only happens if configure decides
9494
# the module is needed (see filter hack in OBJS, above).
9595

96-
chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
96+
chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c syswrap.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
9797
rm -f $@ && $(LN_S) $< .
9898

9999
ip.c md5.c: % : $(backend_src)/libpq/%
@@ -145,7 +145,7 @@ clean distclean: clean-lib
145145
# Might be left over from a Win32 client-only build
146146
rm -f pg_config_paths.h
147147
rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
148-
rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
148+
rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c syswrap.c win32error.c win32setlocale.c
149149
rm -f pgsleep.c
150150
rm -f md5.c ip.c
151151
rm -f encnames.c wchar.c

‎src/interfaces/libpq/bcc32.mak

+7
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ CLEAN :
107107
-@erase "$(INTDIR)\pgsleep.obj"
108108
-@erase "$(INTDIR)\open.obj"
109109
-@erase "$(INTDIR)\system.obj"
110+
-@erase "$(INTDIR)\syswrap.obj"
110111
-@erase "$(INTDIR)\win32error.obj"
111112
-@erase "$(OUTDIR)\$(OUTFILENAME).lib"
112113
-@erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
@@ -151,6 +152,7 @@ LIB32_OBJS= \
151152
"$(INTDIR)\pgsleep.obj" \
152153
"$(INTDIR)\open.obj" \
153154
"$(INTDIR)\system.obj" \
155+
"$(INTDIR)\syswrap.obj" \
154156
"$(INTDIR)\win32error.obj" \
155157
"$(INTDIR)\pthread-win32.obj"
156158

@@ -302,6 +304,11 @@ LINK32_FLAGS = -Gn -L$(BCB)\lib;$(INTDIR); -x -Tpd -v
302304
$(CPP_PROJ) /I"." ..\..\port\system.c
303305
<<
304306

307+
"$(INTDIR)\syswrap.obj" : ..\..\port\syswrap.c
308+
$(CPP) @<<
309+
$(CPP_PROJ) ..\..\port\syswrap.c
310+
<<
311+
305312
"$(INTDIR)\win32error.obj" : ..\..\port\win32error.c
306313
$(CPP) @<<
307314
$(CPP_PROJ) /I"." ..\..\port\win32error.c

‎src/interfaces/libpq/win32.mak

+7
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ CLEAN :
114114
-@erase "$(INTDIR)\pgsleep.obj"
115115
-@erase "$(INTDIR)\open.obj"
116116
-@erase "$(INTDIR)\system.obj"
117+
-@erase "$(INTDIR)\syswrap.obj"
117118
-@erase "$(INTDIR)\win32error.obj"
118119
-@erase "$(INTDIR)\win32setlocale.obj"
119120
-@erase "$(OUTDIR)\$(OUTFILENAME).lib"
@@ -164,6 +165,7 @@ LIB32_OBJS= \
164165
"$(INTDIR)\pgsleep.obj" \
165166
"$(INTDIR)\open.obj" \
166167
"$(INTDIR)\system.obj" \
168+
"$(INTDIR)\syswrap.obj" \
167169
"$(INTDIR)\win32error.obj" \
168170
"$(INTDIR)\win32setlocale.obj" \
169171
"$(INTDIR)\pthread-win32.obj"
@@ -348,6 +350,11 @@ LINK32_OBJS= \
348350
$(CPP_PROJ) /I"." ..\..\port\system.c
349351
<<
350352

353+
"$(INTDIR)\syswrap.obj" : ..\..\port\syswrap.c
354+
$(CPP) @<<
355+
$(CPP_PROJ) ..\..\port\syswrap.c
356+
<<
357+
351358
"$(INTDIR)\win32error.obj" : ..\..\port\win32error.c
352359
$(CPP) @<<
353360
$(CPP_PROJ) /I"." ..\..\port\win32error.c

‎src/pl/plperl/plperl.h

+4-8
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,8 @@
3737
* So we undefine them here and redefine them after it's done its dirty deed.
3838
*/
3939

40-
#ifdef USE_REPL_SNPRINTF
4140
#undef snprintf
4241
#undef vsnprintf
43-
#endif
4442

4543

4644
/* required for perl API */
@@ -49,21 +47,19 @@
4947
#include "XSUB.h"
5048

5149
/* put back our snprintf and vsnprintf */
52-
#ifdef USE_REPL_SNPRINTF
5350
#ifdef snprintf
5451
#undef snprintf
5552
#endif
5653
#ifdef vsnprintf
5754
#undef vsnprintf
5855
#endif
5956
#ifdef __GNUC__
60-
#define vsnprintf(...) pg_vsnprintf(__VA_ARGS__)
61-
#define snprintf(...) pg_snprintf(__VA_ARGS__)
57+
#define vsnprintf(...) vsnprintf_throw_on_fail(__VA_ARGS__)
58+
#define snprintf(...) snprintf_throw_on_fail(__VA_ARGS__)
6259
#else
63-
#define vsnprintf pg_vsnprintf
64-
#define snprintf pg_snprintf
60+
#define vsnprintf vsnprintf_throw_on_fail
61+
#define snprintf snprintf_throw_on_fail
6562
#endif /* __GNUC__ */
66-
#endif /* USE_REPL_SNPRINTF */
6763

6864
/* perl version and platform portability */
6965
#define NEED_eval_pv

‎src/pl/plpython/plpython.h

+4-8
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,8 @@
3535
* So we undefine them here and redefine them after it's done its dirty deed.
3636
*/
3737

38-
#ifdef USE_REPL_SNPRINTF
3938
#undef snprintf
4039
#undef vsnprintf
41-
#endif
4240

4341
#if defined(_MSC_VER) && defined(_DEBUG)
4442
/* Python uses #pragma to bring in a non-default libpython on VC++ if
@@ -125,21 +123,19 @@ typedef int Py_ssize_t;
125123
#include <eval.h>
126124

127125
/* put back our snprintf and vsnprintf */
128-
#ifdef USE_REPL_SNPRINTF
129126
#ifdef snprintf
130127
#undef snprintf
131128
#endif
132129
#ifdef vsnprintf
133130
#undef vsnprintf
134131
#endif
135132
#ifdef __GNUC__
136-
#define vsnprintf(...) pg_vsnprintf(__VA_ARGS__)
137-
#define snprintf(...) pg_snprintf(__VA_ARGS__)
133+
#define vsnprintf(...) vsnprintf_throw_on_fail(__VA_ARGS__)
134+
#define snprintf(...) snprintf_throw_on_fail(__VA_ARGS__)
138135
#else
139-
#define vsnprintf pg_vsnprintf
140-
#define snprintf pg_snprintf
136+
#define vsnprintf vsnprintf_throw_on_fail
137+
#define snprintf snprintf_throw_on_fail
141138
#endif /* __GNUC__ */
142-
#endif /* USE_REPL_SNPRINTF */
143139

144140
/*
145141
* Used throughout, and also by the Python 2/3 porting layer, so it's easier to

‎src/port/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ LIBS += $(PTHREAD_LIBS)
3333
OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
3434
noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
3535
pgstrcasecmp.o pqsignal.o \
36-
qsort.o qsort_arg.o quotes.o sprompt.o tar.o thread.o
36+
qsort.o qsort_arg.o quotes.o sprompt.o syswrap.o tar.o thread.o
3737

3838
# foo_srv.o and foo.o are both built from foo.c, but only foo.o has -DFRONTEND
3939
OBJS_SRV = $(OBJS:%.o=%_srv.o)

0 commit comments

Comments
 (0)