bugGNU nano - Bugs: bug #56902, absolute filenames are...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #56902: absolute filenames are misconcatenated and can then mismatch a syntax

Submitted by:  Brand Huntsman <brand>
Submitted on:  Mon 16 Sep 2019 08:54:59 AM UTC  
 
Severity:  2 - Minor Status:  Fixed
Assigned to:  Benno Schulenberg <bens> Open/Closed:  Closed

Sat 05 Oct 2019 09:45:55 AM UTC, comment #4: 

The fix has been released in nano-4.5.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Tue 17 Sep 2019 11:52:55 AM UTC, comment #3: 

Hmm...  I don't know what I was thinking when wrote that code...  I must have followed Mike's original suggestion in bug #44288, to use getcwd() and canonicalize_file_name(), but then realized that canonicalize_file_name() is a GNU extension and sought a replacement for it.

Anyway, thanks for the suggestion.  I keep the fallback to the relative path, as most syntaxes use a "tail match", not an absolute-path match, so these syntaxes will still work even when an absolute path cannot be determined.  Commit d6e05d83.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Mon 16 Sep 2019 06:05:33 PM UTC, comment #2: 

update_poshistory() calls get_full_path() directly on the filename and produces the same result color_update() is getting with its charalloc/getcwd/sprintf/get_full_path code. Couldn't all of that code be replaced with one line?

fullname = get_full_path(openfile->filename)

(file #47534)

Brand Huntsman <brand>
Mon 16 Sep 2019 03:42:55 PM UTC, comment #1: 

No, there is no reason why color_update() calls get_full_path() for absolute paths, because it is dead wrong to concatenate two absolute paths.  :|  A foolish oversight.

Fixed in git in a simpler way than your patch, commit dbcf4f0c.  Thanks for reporting.

(That there are two unneeded allocations and frees for absolute paths, I don't mind much -- it will not be the most common case.  If we want to make savings, it would be better to call get_full_path() just once for each specified file and store the result in the openfile struct, because currently get_full_path() gets called four times for each specified file. :| )

(I don't think that get_full_path() creates an invalid path -- it's color_update() that created the wrong path, and when the given [absolute] path also exists in the current directory, it is in fact a valid path, so get_full_path() gives it back as okay.)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Mon 16 Sep 2019 08:54:59 AM UTC, original submission:  

Create a new syntax:

syntax foo "^/tmp/foo"
color brightwhite "^.+$"

Create /tmp/foo and verify any content is white.

Create a "tmp" directory in your home directory. If pwd is $HOME, nano won't load the foo syntax when opening /tmp/foo. But change to any other directory that doesn't contain a "tmp" sub-directory, and nano will load the foo syntax for /tmp/foo.

get_full_path() called from color_update() is where the invalid path is generated, but I haven't tracked down where it fails yet.

Is there a reason color_update() calls get_full_path() for absolute paths instead of bypassing all that work and directly using openfile->filename? It doesn't seem to perform any ".." conversion (/tmp/../tmp/foo -> /tmp/foo).

The attached patch bypasses the extra work when the filename begins with a '/'. It fixes the immediate issue, but get_full_path() should still be fixed, as it might break elsewhere in nano.

Brand Huntsman <brand>

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by brand (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-05 bens Open/ClosedOpen => Closed
    2019-09-17 bens Summary[Bug] absolute filenames can become invalid relative paths and not match syntax => absolute filenames are misconcatenated and can then mismatch a syntax
    2019-09-16 brand Attached File- => Added color-call-get_full_path-directly-on-the-filename.patch, #47534
    2019-09-16 bens Severity3 - Normal => 2 - Minor
        StatusNone => Fixed
        Assigned toNone => bens
    2019-09-16 brand Attached File- => Added syntax-don-t-concatenate-filename-with-currentdir-when-filename-is-an-absolute-path.patch, #47526

    Back to the top


    Powered by Savane 3.7