Skip to content

Bounded string parsing in metadata formats to prevent buffer overflows #165

Description

@LucaPalumbo

Description

The file libchdr/include/libchdr/chd.h contains the definition of utility strings such as:

#define CDROM_TRACK_METADATA_FORMAT	"TRACK:%d TYPE:%s SUBTYPE:%s FRAMES:%d"
#define CDROM_TRACK_METADATA2_TAG	CHD_MAKE_TAG('C','H','T','2')
#define CDROM_TRACK_METADATA2_FORMAT	"TRACK:%d TYPE:%s SUBTYPE:%s FRAMES:%d PREGAP:%d PGTYPE:%s PGSUB:%s POSTGAP:%d"
#define GDROM_OLD_METADATA_TAG		CHD_MAKE_TAG('C','H','G','T')
#define GDROM_TRACK_METADATA_TAG	CHD_MAKE_TAG('C', 'H', 'G', 'D')
#define GDROM_TRACK_METADATA_FORMAT	"TRACK:%d TYPE:%s SUBTYPE:%s FRAMES:%d PAD:%d PREGAP:%d PGTYPE:%s PGSUB:%s POSTGAP:%d"

These utility strings are often used by external project to parse header of chd files, using functions like sscanf.

e.g.
in flycast https://github.com/flyinghead/flycast/blob/master/core/imgread/chd.cpp#L156

The problem is that using %s makes the write to the buffers unbounded, risking a buffer overflow.

Proposed solution

A possible fix would be to use %15s, or similar.

#define CDROM_TRACK_METADATA_FORMAT	"TRACK:%d TYPE:%15s SUBTYPE:%15s FRAMES:%d"
#define CDROM_TRACK_METADATA2_TAG	CHD_MAKE_TAG('C','H','T','2')
#define CDROM_TRACK_METADATA2_FORMAT	"TRACK:%d TYPE:%15s SUBTYPE:%15s FRAMES:%d PREGAP:%d PGTYPE:%15s PGSUB:%15s POSTGAP:%d"
#define GDROM_OLD_METADATA_TAG		CHD_MAKE_TAG('C','H','G','T')
#define GDROM_TRACK_METADATA_TAG	CHD_MAKE_TAG('C', 'H', 'G', 'D')
#define GDROM_TRACK_METADATA_FORMAT	"TRACK:%d TYPE:%15s SUBTYPE:%15s FRAMES:%d PAD:%d PREGAP:%d PGTYPE:%15s PGSUB:%15s POSTGAP:%d"

If 15 characters is too restrictive for certain edge-case formats, the limits can be adjusted accordingly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions