Why do the compiler checks .go directory is writeable?

Why do the compiler checks .go directory is writeable?

[rixed]

I'm refering to the ensure-writable-dir function.
I've stumbled upon this problem recently : I have a program that's suid root
but calls guile. The guile compiler then creates some directories in
.cache/guile but then check (with access()) that he can write in there, which
he can't since access revoke the effective uid for the caller uid. We thus have
this situation: the compiler creates a bunch of directories then complains he
cannot write in them.

It's not obvious to me why the compiler should ensure a directory is writable
just to throw an error. Wouldn't it be better to just call opens and writes
and let these fails and report these more acurate errors instead (or, in my
case, let them succeed) ?

Or is it a security concern?

Re: Why do the compiler checks .go directory is writeable?

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

Hi,

Sorry for the delay!

rixed@happyleptic.org skribis:

> I'm refering to the ensure-writable-dir function.
> I've stumbled upon this problem recently : I have a program that's suid root
> but calls guile. The guile compiler then creates some directories in
> .cache/guile but then check (with access()) that he can write in there, which
> he can't since access revoke the effective uid for the caller uid. We thus have
> this situation: the compiler creates a bunch of directories then complains he
> cannot write in them.
>
> It's not obvious to me why the compiler should ensure a directory is writable
> just to throw an error. Wouldn't it be better to just call opens and writes
> and let these fails and report these more acurate errors instead (or, in my
> case, let them succeed) ?

Agreed, not to mention time-of-check-to-time-of-use-errors.

We basically want ‘mkdir -p’, but we can even omit the stat(2) call upon
EEXIST, because if DIR doesn’t point to a directory, the error will be
caught soon after anyway.

I’ll install the following patch if there’s no objection:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2177 bytes --]

diff --git a/module/system/base/compile.scm b/module/system/base/compile.scm
index 0bc11a3..afcb55a 100644
--- a/module/system/base/compile.scm
+++ b/module/system/base/compile.scm
@@ -1,6 +1,6 @@
 ;;; High-level compiler interface
 
-;; Copyright (C) 2001, 2009, 2010, 2011 Free Software Foundation, Inc.
+;; Copyright (C) 2001, 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
 
 ;;; This library is free software; you can redistribute it and/or
 ;;; modify it under the terms of the GNU Lesser General Public
@@ -72,7 +72,7 @@
 ;; before the check, so that we avoid races (possibly due to parallel
 ;; compilation).
 ;;
-(define (ensure-writable-dir dir)
+(define (ensure-directory dir)
   (catch 'system-error
     (lambda ()
       (mkdir dir))
@@ -80,13 +80,12 @@
       (let ((errno (and (pair? rest) (car rest))))
         (cond
          ((eqv? errno EEXIST)
-          (let ((st (stat dir)))
-            (if (or (not (eq? (stat:type st) 'directory))
-                    (not (access? dir W_OK)))
-                (error "directory not writable" dir))))
+          ;; Assume it's a writable directory, to avoid TOCTOU errors,
+          ;; as well as UID/EUID mismatches that occur with access(2).
+          #t)
          ((eqv? errno ENOENT)
-          (ensure-writable-dir (dirname dir))
-          (ensure-writable-dir dir))
+          (ensure-directory (dirname dir))
+          (ensure-directory dir))
          (else
           (throw k subr fmt args rest)))))))
 
@@ -125,7 +124,7 @@
                  %compile-fallback-path
                  (canonical->suffix (canonicalize-path file))
                  (compiled-extension))))
-         (and (false-if-exception (ensure-writable-dir (dirname f)))
+         (and (false-if-exception (ensure-directory (dirname f)))
               f))))
 
 (define* (compile-file file #:key
@@ -144,7 +143,7 @@
       ;; Choose the input encoding deterministically.
       (set-port-encoding! in (or enc "UTF-8"))
 
-      (ensure-writable-dir (dirname comp))
+      (ensure-directory (dirname comp))
       (call-with-output-file/atomic comp
         (lambda (port)
           ((language-printer (ensure-language to))

[-- Attachment #3: Type: text/plain, Size: 18 bytes --]


Thanks,
Ludo’.

Re: Why do the compiler checks .go directory is writeable?

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> We basically want ‘mkdir -p’, but we can even omit the stat(2) call upon
> EEXIST, because if DIR doesn’t point to a directory, the error will be
> caught soon after anyway.
>
> I’ll install the following patch if there’s no objection:

Done, thanks!

Ludo’.

Re: Why do the compiler checks .go directory is writeable?

[rixed]

-[ Tue, Sep 11, 2012 at 11:45:51PM +0200, Ludovic Courtès ]----
> ludo@gnu.org (Ludovic Courtès) skribis:
> 
> > We basically want ???mkdir -p???, but we can even omit the stat(2) call upon
> > EEXIST, because if DIR doesn???t point to a directory, the error will be
> > caught soon after anyway.
> >
> > I???ll install the following patch if there???s no objection:
> 
> Done, thanks!

Thank you Ludo & all for dealing thoroughly any user complains however
unimportant.