Skip site navigation (1) Skip section navigation (2)

7.1 odbc bug & patch

From: Dave Bodenstab <imdave(at)mcs(dot)net>
To: pgsql-patches(at)postgresql(dot)org
Subject: 7.1 odbc bug & patch
Date: 2001-06-06 18:34:31
Message-ID: 200106061834.f56IYV024146@base486.home.org (view raw or flat)
Thread:
Lists: pgsql-patches
Your name               : Dave Bodenstab
Your email address      : imdave(at)mcs(dot)net


System Configuration
---------------------
  Architecture (example: Intel Pentium)         : (shouldn't matter) Intel x86

  Operating System (example: Linux 2.0.26 ELF)  : (shouldn't matter) FreeBSD

  PostgreSQL version (example: PostgreSQL-7.1):   PostgreSQL-7.1

  Compiler used (example:  gcc 2.8.0)           : (shouldn't matter) gcc 2.7.2.1



Please enter a FULL description of your problem:
------------------------------------------------
I installed postgres 7.1 with --enable-odbc.  I then installed
tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
(http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz).  I could not
get either to work... postgres would not find the global odbcinst.ini
file.  I traced this to src/interfaces/odbc/gpps.c -- here are the
many things I think are wrong:

 40  GetPrivateProfileString(char *theSection,		/* section name */
  :
 50  {
 51  	char		buf[MAXPGPATH];
 52  	char	   *ptr = 0;
  :
 63  	int			j = 0;
    
 64  	j = strlen(theIniFileName) + 1;
 65  	ptr = (char *) getpwuid(getuid());	/* get user info */
    
 66  	if (ptr == NULL)
 67  	{
 68  		if (MAXPGPATH - 1 < j)
 69  			theIniFileName[MAXPGPATH - 1] = '\0';
    
 70  		sprintf(buf, "%s", theIniFileName);
 71  	}
 72  	ptr = ((struct passwd *) ptr)->pw_dir;		/* get user home dir */
 73  	if (ptr == NULL || *ptr == '\0')
 74  		ptr = "/home";
    
 75  	/*
 76  	 * This doesn't make it so we find an ini file but allows normal
 77  	 * processing to continue further on down. The likelihood is that the
 78  	 * file won't be found and thus the default value will be returned.
 79  	 */
 80  	if (MAXPGPATH - 1 < strlen(ptr) + j)
 81  	{
 82  		if (MAXPGPATH - 1 < strlen(ptr))
 83  			ptr[MAXPGPATH - 1] = '\0';
 84  		else
 85  			theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
 86  	}
    
 87  	sprintf(buf, "%s/%s", ptr, theIniFileName);
    
 88  	/*
 89  	 * This code makes it so that a file in the users home dir overrides a
 90  	 * the "default" file as passed in
 91  	 */
 92  	aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
 93  	if (!aFile)
 94  	{
 95  		sprintf(buf, "%s", theIniFileName);
 96  		aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
 97  	}
       
+ Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
  ptr anyway.  Then line 73 checks for NULL again!?
+ Line 87 unconditionally ignores what line 70 did.
+ Lines 69, 83 and 85 modify the value passed to the function unbeknownst
  to the calling function.  The values could be a static string or might
  be further used by the calling function.
+ Truncating the strings might result in a string that accidently matches
  another, totally non-related file.
+ Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
  be true.
+ The globally installed odbcinst.ini file is never checked.  This is
  why I couldn't get things to work -- it was looking for odbcinst.ini
  only in my home directory or the current directory.



Please describe a way to repeat the problem.   Please try to provide a
concise reproducible example, if at all possible: 
----------------------------------------------------------------------
Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
global ini file is installed.)  The result is always the error
message that ``one of server,port,database,etc. are missing''.

Run libiodbc-2.50.3/samples/odbctest <DSNname>.  The command fails
to connect to the database and just exits.



If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------
I couldn't find any documentation as to what the correct behavior should
be, and the comments aren't any clearer.  I changed the code to look for
the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
globally installed file.  I also fixed all the problems I mentioned
above.  Here is the patch, it works for me.  Use it any way you want.

--- gpps.c	2001/06/06 02:53:05	7.1
+++ gpps.c	2001/06/06 17:23:39	7.1.1.3
@@ -35,6 +35,7 @@
 #include <string.h>
 #include "misc.h"
 #include "gpps.h"
+#include "dlg_specific.h"
 
 #ifndef TRUE
 #define TRUE	((BOOL)1)
@@ -44,6 +45,12 @@
 #endif
 
 
+/*
+ * theIniFileName is searched for in:
+ * 	$HOME/theIniFileName
+ * 	theIniFileName
+ * 	ODBCINST_INI
+ */
 DWORD
 GetPrivateProfileString(char *theSection,		/* section name */
 						char *theKey,	/* search key name */
@@ -68,48 +75,36 @@
 	size_t		aReturnLength = 0;
 	BOOL		aSectionFound = FALSE;
 	BOOL		aKeyFound = FALSE;
-	int			j = 0;
 
-	j = strlen(theIniFileName) + 1;
 	ptr = (char *) getpwuid(getuid());	/* get user info */
 
-	if (ptr == NULL)
-	{
-		if (MAXPGPATH - 1 < j)
-			theIniFileName[MAXPGPATH - 1] = '\0';
-
-		sprintf(buf, "%s", theIniFileName);
-	}
-	ptr = ((struct passwd *) ptr)->pw_dir;		/* get user home dir */
-	if (ptr == NULL || *ptr == '\0')
+	if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
 		ptr = "/home";
+	else
+		ptr = ((struct passwd *) ptr)->pw_dir;		/* get user home dir */
 
 	/*
-	 * This doesn't make it so we find an ini file but allows normal
-	 * processing to continue further on down. The likelihood is that the
-	 * file won't be found and thus the default value will be returned.
+	 * If it can't be opened because the paths are too long, then
+	 * skip it, don't just truncate the path string...  The truncated path
+	 * might accidently be an existing file.  The default value will be
+	 * returned instead.
 	 */
-	if (MAXPGPATH - 1 < strlen(ptr) + j)
+	if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
 	{
-		if (MAXPGPATH - 1 < strlen(ptr))
-			ptr[MAXPGPATH - 1] = '\0';
-		else
-			theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
+		sprintf(buf, "%s/%s", ptr, theIniFileName);
+		aFile = (FILE *) fopen(buf, PG_BINARY_R);
 	}
 
-	sprintf(buf, "%s/%s", ptr, theIniFileName);
-
 	/*
 	 * This code makes it so that a file in the users home dir overrides a
 	 * the "default" file as passed in
 	 */
-	aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
 	if (!aFile)
 	{
-		sprintf(buf, "%s", theIniFileName);
-		aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
+		aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
+		if (!aFile)
+			aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
 	}
-
 
 	aLength = (theDefault == NULL) ? 0 : strlen(theDefault);
 




Responses

pgsql-patches by date

Next:From: Jason TishlerDate: 2001-06-06 18:38:24
Subject: Re: YA readline 4.2 patch
Previous:From: Tom LaneDate: 2001-06-06 18:08:21
Subject: Re: YA readline 4.2 patch

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group