cpfind always fail on photos with long path

Bug #1057012 reported by gouri on 2012-09-26
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libpano13 (Ubuntu)

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: =========

## 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
  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)

gouri (stephane-gourichon-lpad) wrote :
gouri (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 :

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 );
                    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.

Fabrice Coutadeur (fabricesp) wrote :


You're totally right! Assigning to libpano13 package.



affects: hugin (Ubuntu) → libpano13 (Ubuntu)
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) on 2014-01-01
Changed in panotools:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers