[PATCH] Modify mingw_main() workaround to avoid link errors

With MinGW's

   gcc.exe (GCC) 3.4.5 (mingw special)
   GNU ld version 2.17.50 20060824

the old define caused link errors:

   git.o: In function `main':
   C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
   collect2: ld returned 1 exit status

The modified define works.

Signed-off-by: Steffen Prohaska <redacted>
---
 compat/mingw.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 290a9e6..a52e657 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h

@@ -228,9 +228,10 @@ char **env_setenv(char **env, const char *name);
  * A replacement of main() that ensures that argv[0] has a path
  */
 
-#define main(c,v) main(int argc, const char **argv) \
+#define main(c,v) dummy_decl_mingw_main(); \
+static int mingw_main(); \
+int main(int argc, const char **argv) \
 { \
-	static int mingw_main(); \
 	argv[0] = xstrdup(_pgmptr); \
 	return mingw_main(argc, argv); \
 } \

-- 
1.6.0.rc0.42.g186458

Hi,

On Sat, 26 Jul 2008, Steffen Prohaska wrote:

-#define main(c,v) main(int argc, const char **argv) \
+#define main(c,v) dummy_decl_mingw_main(); \

What is this dummy_*() statement supposed to do?

Note that I still think it would be a better fix to refactor the 
lookup_prog() function from mingw.c.

Ciao,
Dscho

When the program 'git' is in the PATH, the argv[0] is set to the basename.
However, argv0_path needs the full path, so add a function to discover the
program by traversing the PATH manually.

Signed-off-by: Johannes Schindelin <redacted>
---

	So it is not easily possible to reuse this function in 
	compat/mingw.c, as Junio said that compat/ should not depend
	(at least too much) on libgit.a.

	Of course, we could try to follow a symlinked git, too, but I 
	think this is overkill until someone proves me wrong.

 exec_cmd.c |   22 ++++++++++++++++++++++
 exec_cmd.h |    1 +
 git.c      |    6 ++++++
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 0ed768d..048f3ca 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c

@@ -125,3 +125,25 @@ int execl_git_cmd(const char *cmd,...)
 	argv[argc] = NULL;
 	return execv_git_cmd(argv);
 }
+
+char *lookup_program_in_path(const char *program)
+{
+	struct strbuf buf = STRBUF_INIT;
+	const char *path = getenv("PATH");
+
+	if (!path || !*path)
+		return NULL;
+
+	for (;;) {
+		const char *colon = strchrnul(path, PATH_SEP);
+
+		strbuf_setlen(&buf, 0);
+		strbuf_addf(&buf, "%.*s/%s",
+				(int)(colon - path), path, program);
+		if (!access(buf.buf, X_OK))
+			return strbuf_detach(&buf, NULL);
+		if (!*colon)
+			return NULL;
+		path = colon + 1;
+	}
+}

diff --git a/exec_cmd.h b/exec_cmd.h
index 0c46cd5..4548390 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h

@@ -8,5 +8,6 @@ extern void setup_path(void);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 extern const char *system_path(const char *path);
+extern char *lookup_program_in_path(const char *program);
 
 #endif /* GIT_EXEC_CMD_H */

diff --git a/git.c b/git.c
index 54c5bfa..0ec8ee1 100644
--- a/git.c
+++ b/git.c

@@ -428,6 +428,12 @@ int main(int argc, const char **argv)
 	do
 		--slash;
 	while (cmd <= slash && !is_dir_sep(*slash));
+	if (slash < cmd) {
+		cmd = lookup_program_in_path(cmd);
+		for (slash = (char *)cmd + strlen(cmd) - 1;
+				cmd <= slash && !is_dir_sep(*slash); slash--)
+			; /* do nothing */
+	}
 	if (cmd <= slash) {
 		*slash++ = 0;
 		git_set_argv0_path(cmd);

-- 
1.5.6.2.516.g22071

On 26-07-08 16:14, Johannes Schindelin wrote:

When the program 'git' is in the PATH, the argv[0] is set to the
basename. However, argv0_path needs the full path, so add a function
to discover the program by traversing the PATH manually.

While not having read the context for this, this ofcourse sounds like a 
huge gaping race-condition. If applicable here (as said, did not read 
context) you generally want to make sure that there's no window that a 
path could be replaced -- while perhaps not here, that's often the kind 
of thing that security attacks end up abusing.

Rene.

Hi,

On Sat, 26 Jul 2008, Rene Herman wrote:

On 26-07-08 16:14, Johannes Schindelin wrote:

quoted

When the program 'git' is in the PATH, the argv[0] is set to the
basename. However, argv0_path needs the full path, so add a function
to discover the program by traversing the PATH manually.

While not having read the context for this, this ofcourse sounds like a huge
gaping race-condition. If applicable here (as said, did not read context) you
generally want to make sure that there's no window that a path could be
replaced -- while perhaps not here, that's often the kind of thing that
security attacks end up abusing.

Yeah, and that's why you would carefully time your attack just in between 
the command invocation and the discovery of argv[0] in the PATH.

Rather than replacing the 'git' program with an infected version right 
away.

Giggling,
Dscho

On 26-07-08 17:10, Johannes Schindelin wrote:

Hi,

On Sat, 26 Jul 2008, Rene Herman wrote:

quoted

On 26-07-08 16:14, Johannes Schindelin wrote:

quoted

When the program 'git' is in the PATH, the argv[0] is set to the
basename. However, argv0_path needs the full path, so add a function
to discover the program by traversing the PATH manually.

While not having read the context for this, this ofcourse sounds like a huge
gaping race-condition. If applicable here (as said, did not read context) you
generally want to make sure that there's no window that a path could be
replaced -- while perhaps not here, that's often the kind of thing that
security attacks end up abusing.

Yeah, and that's why you would carefully time your attack just in between 
the command invocation and the discovery of argv[0] in the PATH.

Rather than replacing the 'git' program with an infected version right 
away.

Adding to the PATH is generally not disallowed by user level security. 
Replacing the GIT binary generally is.

Sure maybe it's not much of a problem here; as said, I didn't read the 
context and am not a GIT person. Just commented on a git-user list when 
this was the next message on the list. Though a heads-up might still be 
in order. If it wasn't useful -- so be it, but even making a command do 
something different than a user expected can have serious implications, 
for example in this case for the tree they are working on.

Rene.

Hi,

On Sat, 26 Jul 2008, Rene Herman wrote:

Adding to the PATH is generally not disallowed by user level security. 
Replacing the GIT binary generally is.

Prepending to the PATH is generally not disallowed either.  And that's 
just as good as replacing the Git binary.

This issue is totally independent of Git.  And it is totally bogus to 
think about the complicated issues when the "weakest link of the chain" is 
much easier to exploit.

Hth,
Dscho

On 26-07-08 17:35, Johannes Schindelin wrote:

And it is totally bogus to think about the complicated issues when
the "weakest link of the chain" is much easier to exploit.

/me tips hat and unsubscribes again.

Rene.

On Jul 26, 2008, at 3:17 PM, Johannes Schindelin wrote:

On Sat, 26 Jul 2008, Steffen Prohaska wrote:

quoted

-#define main(c,v) main(int argc, const char **argv) \
+#define main(c,v) dummy_decl_mingw_main(); \

What is this dummy_*() statement supposed to do?

Avoid compile errors.  The original statement is

    int main( ...

But we want

    static int mingw_main( ...

So we need to first get rid of the original int, before
we can start the static decl.  We get rid by completing
the original int with the dummy_decl_mingw_main(); to a
full function decl.

	Steffen

Zitat von Steffen Prohaska [off-list ref]:

With MinGW's

   gcc.exe (GCC) 3.4.5 (mingw special)
   GNU ld version 2.17.50 20060824

the old define caused link errors:

   git.o: In function `main':
   C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
   collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

-- Hannes

On Jul 26, 2008, at 10:37 PM, Johannes Sixt wrote:

Zitat von Steffen Prohaska [off-list ref]:

quoted

With MinGW's

  gcc.exe (GCC) 3.4.5 (mingw special)
  GNU ld version 2.17.50 20060824

the old define caused link errors:

  git.o: In function `main':
  C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
  collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

I cleaned my work tree and built several times but did not
find out what exactly is causing the error.  So I came up
with the modified define, which declares the static
mingw_main in global scope.  I have no clue why I see the
error that you don't have.

	Steffen

Zitat von Steffen Prohaska [off-list ref]:

On Jul 26, 2008, at 10:37 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

With MinGW's

  gcc.exe (GCC) 3.4.5 (mingw special)
  GNU ld version 2.17.50 20060824

the old define caused link errors:

  git.o: In function `main':
  C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
  collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

I cleaned my work tree and built several times but did not
find out what exactly is causing the error.  So I came up
with the modified define, which declares the static
mingw_main in global scope.  I have no clue why I see the
error that you don't have.

Neither do I. But a strange line number you have there. In 01d9b2d (from
mingw.git) I have 'exit(1)' in line 500 of git.c.

-- Hannes

On Jul 27, 2008, at 9:24 PM, Johannes Sixt wrote:

Zitat von Steffen Prohaska [off-list ref]:

quoted

On Jul 26, 2008, at 10:37 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

With MinGW's

 gcc.exe (GCC) 3.4.5 (mingw special)
 GNU ld version 2.17.50 20060824

the old define caused link errors:

 git.o: In function `main':
 C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
 collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

I cleaned my work tree and built several times but did not
find out what exactly is causing the error.  So I came up
with the modified define, which declares the static
mingw_main in global scope.  I have no clue why I see the
error that you don't have.

Neither do I. But a strange line number you have there. In 01d9b2d  
(from
mingw.git) I have 'exit(1)' in line 500 of git.c.

I have the same in line 500.  I am still wondering what this could
mean.  But I do not yet now :-(

	Steffen

Zitat von Steffen Prohaska [off-list ref]:

On Jul 27, 2008, at 9:24 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

On Jul 26, 2008, at 10:37 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

With MinGW's

 gcc.exe (GCC) 3.4.5 (mingw special)
 GNU ld version 2.17.50 20060824

the old define caused link errors:

 git.o: In function `main':
 C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
 collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

I cleaned my work tree and built several times but did not
find out what exactly is causing the error.  So I came up
with the modified define, which declares the static
mingw_main in global scope.  I have no clue why I see the
error that you don't have.

Neither do I. But a strange line number you have there. In 01d9b2d
(from
mingw.git) I have 'exit(1)' in line 500 of git.c.

I have the same in line 500.  I am still wondering what this could
mean.  But I do not yet now :-(

Can you try 'make -k' and see whether you have a similar problem with the
non-builtins that have their own main()?

-- Hannes

On Jul 29, 2008, at 10:33 AM, Johannes Sixt wrote:

Zitat von Steffen Prohaska [off-list ref]:

quoted

On Jul 27, 2008, at 9:24 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

On Jul 26, 2008, at 10:37 PM, Johannes Sixt wrote:

quoted

Zitat von Steffen Prohaska [off-list ref]:

quoted

With MinGW's

gcc.exe (GCC) 3.4.5 (mingw special)
GNU ld version 2.17.50 20060824

the old define caused link errors:

git.o: In function `main':
C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
collect2: ld returned 1 exit status

The modified define works.

I have the same tools, but not this error. ???

I cleaned my work tree and built several times but did not
find out what exactly is causing the error.  So I came up
with the modified define, which declares the static
mingw_main in global scope.  I have no clue why I see the
error that you don't have.

Neither do I. But a strange line number you have there. In 01d9b2d
(from
mingw.git) I have 'exit(1)' in line 500 of git.c.

I have the same in line 500.  I am still wondering what this could
mean.  But I do not yet now :-(

Can you try 'make -k' and see whether you have a similar problem  
with the
non-builtins that have their own main()?

With your master 01d9b2d:

$ make -k
     LINK git.exe
git.o: In function `main':
C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git.exe] Error 1
     LINK git-hash-object.exe
hash-object.o: In function `main':
C:/msysgit/git/hash-object.c:114: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-hash-object.exe] Error 1
     LINK git-index-pack.exe
index-pack.o: In function `main':
C:/msysgit/git/index-pack.c:974: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-index-pack.exe] Error 1
     LINK git-merge-index.exe
merge-index.o: In function `main':
C:/msysgit/git/merge-index.c:120: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-merge-index.exe] Error 1
     LINK git-merge-tree.exe
merge-tree.o: In function `main':
C:/msysgit/git/merge-tree.c:346: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-merge-tree.exe] Error 1
     LINK git-mktag.exe
mktag.o: In function `main':
C:/msysgit/git/mktag.c:144: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-mktag.exe] Error 1
     LINK git-mktree.exe
mktree.o: In function `main':
C:/msysgit/git/strbuf.h:73: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-mktree.exe] Error 1
     LINK git-pack-redundant.exe
pack-redundant.o: In function `main':
C:/msysgit/git/pack-redundant.c:181: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-pack-redundant.exe] Error 1
     LINK git-patch-id.exe
patch-id.o: In function `main':
C:/msysgit/git/patch-id.c:80: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-patch-id.exe] Error 1
     LINK git-receive-pack.exe
receive-pack.o: In function `main':
C:/msysgit/git/receive-pack.c:386: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-receive-pack.exe] Error 1
     LINK git-show-index.exe
show-index.o: In function `main':
C:/msysgit/git/show-index.c:64: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-show-index.exe] Error 1
     LINK git-unpack-file.exe
unpack-file.o: In function `main':
C:/msysgit/git/unpack-file.c:19: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-unpack-file.exe] Error 1
     LINK git-update-server-info.exe
update-server-info.o: In function `main':
C:/msysgit/git/update-server-info.c:20: undefined reference to  
`mingw_main'
collect2: ld returned 1 exit status
make: *** [git-update-server-info.exe] Error 1
     LINK git-upload-pack.exe
upload-pack.o: In function `main':
C:/msysgit/git/upload-pack.c:180: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-upload-pack.exe] Error 1
     LINK git-var.exe
var.o: In function `main':
C:/msysgit/git/var.c:51: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [git-var.exe] Error 1
make: Target `all' not remade because of errors.
     SUBDIR git-gui
     SUBDIR gitk-git
make[1]: Nothing to be done for `all'.
     SUBDIR perl
mkdir -p blib/lib
rm -f blib/lib/Git.pm; cp Git.pm blib/lib/
rm -f blib/lib/Error.pm
     SUBDIR templates
     LINK test-chmtime.exe
test-chmtime.o: In function `main':
C:/msysgit/git/test-chmtime.c:50: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [test-chmtime.exe] Error 1
     LINK test-date.exe
test-date.o: In function `main':
C:/msysgit/git/test-date.c:3: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [test-date.exe] Error 1
     LINK test-delta.exe
test-delta.o: In function `main':
C:/msysgit/git/test-delta.c:67: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [test-delta.exe] Error 1
     LINK test-sha1.exe
test-sha1.o: In function `main':
C:/msysgit/git/test-sha1.c:14: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [test-sha1.exe] Error 1
     LINK test-match-trees.exe
test-match-trees.o: In function `main':
C:/msysgit/git/test-match-trees.c:23: undefined reference to  
`mingw_main'
collect2: ld returned 1 exit status
make: *** [test-match-trees.exe] Error 1
     LINK test-parse-options.exe
test-parse-options.o: In function `main':
C:/msysgit/git/test-parse-options.c:21: undefined reference to  
`mingw_main'
collect2: ld returned 1 exit status
make: *** [test-parse-options.exe] Error 1
     LINK test-path-utils.exe
test-path-utils.o: In function `main':
C:/msysgit/git/test-path-utils.c:8: undefined reference to `mingw_main'
collect2: ld returned 1 exit status
make: *** [test-path-utils.exe] Error 1
make: Target `all' not remade because of errors.

	Steffen

Zitat von Steffen Prohaska [off-list ref]:

With MinGW's

   gcc.exe (GCC) 3.4.5 (mingw special)
   GNU ld version 2.17.50 20060824

the old define caused link errors:

   git.o: In function `main':
   C:/msysgit/git/git.c:500: undefined reference to `mingw_main'
   collect2: ld returned 1 exit status

The modified define works.

Signed-off-by: Steffen Prohaska <redacted>

Acked-by: Johannes Sixt <redacted>

I was not aware that my version (block-scoped static function forward
declaration) is not valid C. Thanks, Björn, for pointing out the gcc bugzilla
entries.

-- Hannes

On Sat, Jul 26, 2008 at 16:14:33 +0200, Johannes Schindelin wrote:

When the program 'git' is in the PATH, the argv[0] is set to the basename.
However, argv0_path needs the full path, so add a function to discover the
program by traversing the PATH manually.

Signed-off-by: Johannes Schindelin <redacted>
---

	So it is not easily possible to reuse this function in 
	compat/mingw.c, as Junio said that compat/ should not depend
	(at least too much) on libgit.a.

	Of course, we could try to follow a symlinked git, too, but I 
	think this is overkill until someone proves me wrong.

On UNIX, not only that argv[0] can contain the program without path -- it can
contain anything the user thinks of. However most systems provide some way to
get the path of the executable. On Linux (and some other unices, but not all
of them) a reliable way is to readlink("/proc/self/exe", ...). Maybe since
it's only needed for resolving a relative exec dir, relative exec dir could
be supported only on systems that have such method (which is most of them).

-- 
						 Jan 'Bulb' Hudec [off-list ref]