cpfind always fail on photos with long path

Bug #1057012 reported by Stéphane Gourichon
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Panotools
Fix Released
Undecided
Unassigned
libpano13 (Ubuntu)
New
Undecided
Unassigned

Bug Description

# Summary

When run with long paths, cpfind always fails.
Impact : no automatic control points are available in hugin.

# Symptom

When hugin is instructed to work on photos that have a long absolute
filesystem path, hugin never finds any control point using cpfind.
Instead the default popup says :

> Warning n unconnected image groups found: (list of [imagenumber])
> Please create control points between unconnected images using the Control Points tab.
>
> After adding the points, press the "Align" button again

I figured out it was a path length problem because when making a minimal test case with shorter path, the bug disappears.

# How to reproduce

Make a directory with a long path, e.g.

MP=~/AiHome/gros/tries/30_perso/image/PhotoNumerique/by_year/2012-01-02_11.46.57_2012-09-25_18.34.24_Annee_2012/panocuisine/2012-09-25_18.29.02_2012-09-25_18.29.07_correction_porte_fermee_NET ; mkdir -p $MP ; cd $MP

* Copy at least two JPEGs from a digital camera there. Names can be e.g. 2012-09-25_18.29.03__DSC_3466.JPG
* Open hugin
* Import two photos from that directory
* Press "Align..."

## Expected

* Some control points found, depending on photos.

## Observed

* No control point found.
* cpfind log (obtained before window disappears, or by running it separately) says

--- Find matches ---
*** buffer overflow detected ***: cpfind terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x2b29d619cee7]
/lib/x86_64-linux-gnu/libc.so.6(+0x107de0)[0x2b29d619bde0]
/lib/x86_64-linux-gnu/libc.so.6(+0x107249)[0x2b29d619b249]
/lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0xdd)[0x2b29d610faad]
/lib/x86_64-linux-gnu/libc.so.6(_IO_vfprintf+0x256e)[0x2b29d60ddbae]
/lib/x86_64-linux-gnu/libc.so.6(__vsprintf_chk+0x94)[0x2b29d619b2e4]
/lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x2b29d619b22d]
/usr/lib/libpano13.so.2(ParseScript+0x7f6)[0x2b29d51fe536]
/usr/lib/hugin/libhuginbase.so.0.0(_ZN9HuginBase6PTools8optimizeERNS_12PanoramaDataEPKc+0x46)[0x2b29d4cbfcc6]
/usr/lib/hugin/libhuginbase.so.0.0(_ZN6Ransac7computeIN9HuginBase14PTOptEstimatorESt6vectorIdSaIdEENS1_12ControlPointEEES3_IPKT1_SaIS9_EERT0_RS3_IiSaIiEERKT_RKS3_IS7_SaIS7_EEdd+0x124a)[0x2b29d477106a]
/usr/lib/hugin/libhuginbase.so.0.0(_ZN9HuginBase15RANSACOptimizer11findInliersERNS_12PanoramaDataEiidNS0_4ModeE+0x166)[0x2b29d4769996]
cpfind(_ZN12PanoDetector22RansacMatchesInPairCamERNS_9MatchDataERKS_+0x592)[0x44e7c2]
cpfind(_ZN17MatchDataRunnable3runEv+0x35)[0x443805]
/usr/lib/libZThread-2.3.so.2(+0x186ce)[0x2b29d3eee6ce]
/usr/lib/libZThread-2.3.so.2(+0x17062)[0x2b29d3eed062]
/usr/lib/libZThread-2.3.so.2(_ZN7ZThread10ThreadImpl8dispatchEPS0_S1_NS_4TaskE+0x2ed)[0x2b29d3efb66d]
/usr/lib/libZThread-2.3.so.2(+0x25b26)[0x2b29d3efbb26]
/usr/lib/libZThread-2.3.so.2(_dispatch+0xa)[0x2b29d3efefba]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x7e9a)[0x2b29d4111e9a]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x2b29d618639d]

## Additional information

* When shortening path, problem disappears.
* (Separate issue) perhaps hugin should have spotted failure of cpfind instead of just saying no control point was found.
* I can provide some photos if needed.

1) The release of Ubuntu you are using, via 'lsb_release -rd' or System -> About Ubuntu

$ lsb_release -rd
Description: Ubuntu 12.04.1 LTS
Release: 12.04

2) The version of the package you are using, via 'apt-cache policy pkgname' or by checking in Software Center

$ LC_ALL=C apt-cache policy hugin
hugin:
  Installed: 2011.4.0+dfsg-1
  Candidate: 2011.4.0+dfsg-1
  Version table:
 *** 2011.4.0+dfsg-1 0
        500 http://fr.archive.ubuntu.com/ubuntu/ precise/universe amd64 Packages
        100 /var/lib/dpkg/status

ProblemType: Bug
DistroRelease: Ubuntu 12.04
Package: hugin 2011.4.0+dfsg-1
ProcVersionSignature: Ubuntu 3.2.0-31.50-generic 3.2.28
Uname: Linux 3.2.0-31-generic x86_64
ApportVersion: 2.0.1-0ubuntu13
Architecture: amd64
Date: Wed Sep 26 16:34:42 2012
InstallationMedia: Ubuntu 12.04 LTS "Precise Pangolin" - Beta amd64 (20120419)
SourcePackage: hugin
UpgradeStatus: No upgrade log present (probably fresh install)

Revision history for this message
Stéphane Gourichon (stephane-gourichon-lpad) wrote :
Revision history for this message
Stéphane Gourichon (stephane-gourichon-lpad) wrote :

# Summary

* found the actual bug location, in libpano13.
* bug class : unchecked write to fixed size buffer (buffers have hardcoded size)
* hard-coded limits are inconsistent between files (source buffer 65536, destination buffer 256)
* easy to fix ? There is at least the quick-and-easy by increasing lower limit.

## Additional information

It's in libpano13, file panorama.h, line 413 :

#define PANO_PATH_LEN 255

In a nutshell, ParseScript can parse lines up to 65535 characters long, but Image structure only accepts full paths up to 256 characters long.

## Investigation details

crash log says :
/lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x2b29d619b22d]
/usr/lib/libpano13.so.2(ParseScript+0x7f6)[0x2b29d51fe536]

ParseScript is therefore a function in libpano13.
apt-get source libpano13
cd libpano13-2.9.18+dfsg/

ParseScript is defined in parser.c.
It calls sprintf on line 448

                    case 'n': // Set filename
                        nextWord( buf, &li );
                        sprintf( im->name, "%s", buf );
                        break;
                    case 'm': // Frame

buf is defined on line 148:

    char *li, line[LINE_LENGTH], *ch ,*lineStart, buf[LINE_LENGTH];

buf is big enough to hold a long filename :

//Increased so more params can be parsed/optimized (MRDL - March 2002)
#define LINE_LENGTH 65536

Now check im->name.

In ParseScript, im is defined on line 142:

Image *im;

Image type is defined in panorama.h on line 430-355:

struct Image
{
    // Pixel data
    pt_int32 width;
    pt_int32 height;
    pt_int32 bytesPerLine;
    pt_int32 bitsPerPixel; // Must be 24 or 32
    size_t dataSize;
    unsigned char **data;
    pt_int32 dataformat; // rgb, Lab etc
    pt_int32 format; // Projection: rectilinear etc
    int formatParamCount; // Number of format parameters.
    double formatParam[PANO_PROJECTION_MAX_PARMS]; // Parameters for format.
    int precomputedCount; // number of values precomputed for a given pano
    double precomputedValue[PANO_PROJECTION_PRECOMPUTED_VALUES]; // to speed up pano creation
    double hfov;
    double yaw;
    double pitch;
    double roll;
    cPrefs cP; // How to correct the image
    char name[PANO_PATH_LEN+1];
    PTRect selection;
    CropInfo cropInformation; // TO BE DEPRECATED

    pano_ImageMetadata metadata;
};

typedef struct Image Image;

field "name" is on line 455:

    char name[PANO_PATH_LEN+1];

PANO_PATH_LEN is defined on panorama.h, line 413:

#define PANO_PATH_LEN 255

Crash is explained.

Revision history for this message
Fabrice Coutadeur (fabricesp) wrote :

Hi,

You're totally right! Assigning to libpano13 package.

Thanks,

Fabrice

affects: hugin (Ubuntu) → libpano13 (Ubuntu)
Revision history for this message
Andreas Metzler (k-launchpad-downhill-at-eu-org) wrote :
Revision history for this message
tmodes (tmodes) wrote :

Tried to fix in repository.
We can't increase PANO_PATH_LEN unconditionally, because on Windows the path length is limited (or you use a different function, which would require some more changes). So using instead MAX_PATH_LENGTH, which is already used by libpano13 and is 512 on *nix systems. This hopefully fixes the crash.

Changed in panotools:
status: New → Fix Committed
tmodes (tmodes)
Changed in panotools:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.