Skip to content

Commit

Permalink
fixed many security issues with the too crude mp4 reader
Browse files Browse the repository at this point in the history
  • Loading branch information
@dnewman-gpsw
dnewman-gpsw committed May 28, 2019
1 parent 3e7d6ea commit 341f12c
Show file tree
Hide file tree
Showing 5 changed files with 517 additions and 344 deletions.
4 changes: 2 additions & 2 deletions GPMF_parser.c
View file
Expand Up @@ -2,7 +2,7 @@
*
* @brief GPMF Parser library
*
* @version 1.2.1
* @version 1.2.2
*
* (C) Copyright 2017 GoPro Inc (http://gopro.com/).
*
Expand Down Expand Up @@ -42,7 +42,7 @@ GPMF_ERR IsValidSize(GPMF_stream *ms, uint32_t size) // size is in longs not byt
{
if (ms)
{
int32_t nestsize = (int32_t)ms->nest_size[ms->nest_level];
uint32_t nestsize = (uint32_t)ms->nest_size[ms->nest_level];
if (nestsize == 0 && ms->nest_level == 0)
nestsize = ms->buffer_size_longs;

Expand Down
8 changes: 6 additions & 2 deletions GPMF_parser.h
View file
Expand Up @@ -2,7 +2,7 @@
*
* @brief GPMF Parser library include
*
* @version 1.1.0
* @version 1.1.1
*
* (C) Copyright 2017 GoPro Inc (http://gopro.com/).
*
Expand Down Expand Up @@ -126,7 +126,11 @@ typedef enum GPMFKey // TAG in all caps are GoPro preserved (are defined by GoPr
GPMF_KEY_UNITS = MAKEID('U','N','I','T'),//UNIT - Freedform display string for metadata units (char sting like "RPM", "MPH", "km/h", etc)
GPMF_KEY_SCALE = MAKEID('S','C','A','L'),//SCAL - divisor for input data to scale to the correct units.
GPMF_KEY_TYPE = MAKEID('T','Y','P','E'),//TYPE - Type define for complex data structures
GPMF_KEY_TOTAL_SAMPLES = MAKEID('T','S','M','P'),//TOTL - Total Sample Count including the current payload
GPMF_KEY_TOTAL_SAMPLES = MAKEID('T','S','M','P'),//TSMP - Total Sample Count including the current payload
GPMF_KEY_TIME_OFFSET = MAKEID('T','I','M','O'),//TIMO - Time offset of the metadata stream that follows (single 4 byte float)
GPMF_KEY_TIMING_OFFSET = MAKEID('T','I','M','O'),//TIMO - duplicated, as older code might use the other version of TIMO
GPMF_KEY_TIME_STAMP = MAKEID('S','T','M','P'),//STMP - Time stamp for the first sample.
GPMF_KEY_TIME_STAMPS = MAKEID('S','T','P','S'),//STPS - Stream of all the timestamps delivered (Generally don't use this. This would be if your sensor has no peroidic times, yet precision is required, or for debugging.)
GPMF_KEY_TICK = MAKEID('T','I','C','K'),//TICK - Used for slow data. Beginning of data timing in milliseconds.
GPMF_KEY_TOCK = MAKEID('T','O','C','K'),//TOCK - Used for slow data. End of data timing in milliseconds.
GPMF_KEY_EMPTY_PAYLOADS = MAKEID('E','M','P','T'),//EMPT - Payloads that are empty since the device start (e.g. BLE disconnect.)
Expand Down
13 changes: 10 additions & 3 deletions demo/GPMF_demo.c
View file
Expand Up @@ -46,6 +46,12 @@ int main(int argc, char *argv[])
}

size_t mp4 = OpenMP4Source(argv[1], MOV_GPMF_TRAK_TYPE, MOV_GPMF_TRAK_SUBTYPE);
if (mp4 == 0)
{
printf("error: %s is an invalid MP4/MOV\n", argv[1]);
return -1;
}

// size_t mp4 = OpenMP4SourceUDTA(argv[1]); //Search for GPMF payload with MP4's udta

metadatalength = GetDuration(mp4);
Expand Down Expand Up @@ -90,7 +96,7 @@ int main(int argc, char *argv[])
for (index = 0; index < payloads; index++)
{
uint32_t payloadsize = GetPayloadSize(mp4, index);
float in = 0.0, out = 0.0; //times
double in = 0.0, out = 0.0; //times
payload = GetPayload(mp4, payload, index);
if (payload == NULL)
goto cleanup;
Expand Down Expand Up @@ -238,9 +244,10 @@ int main(int argc, char *argv[])
{
if (GPMF_OK == GPMF_SeekToSamples(ms)) //find the last FOURCC within the stream
{
double in = 0.0, out = 0.0;
uint32_t fourcc = GPMF_Key(ms);
double rate = GetGPMFSampleRate(mp4, fourcc, GPMF_SAMPLE_RATE_PRECISE);// GPMF_SAMPLE_RATE_FAST);
printf("%c%c%c%c sampling rate = %f Hz\n", PRINTF_4CC(fourcc), rate);
double rate = GetGPMFSampleRate(mp4, fourcc, GPMF_SAMPLE_RATE_PRECISE, &in, &out);// GPMF_SAMPLE_RATE_FAST);
printf("%c%c%c%c sampling rate = %f Hz (from %f to %f)\n", PRINTF_4CC(fourcc), rate, in, out);
}
}
#endif
Expand Down

2 comments on commit 341f12c

@hongxuchen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but this commit suddenly makes gcc-7.3/clang-4.0/clang-7 build fails.

[1/5] Building C object CMakeFiles/gpmf-parser.dir/demo/GPMF_mp4reader.c.o
FAILED: CMakeFiles/gpmf-parser.dir/demo/GPMF_mp4reader.c.o
/usr/bin/cc    -MD -MT CMakeFiles/gpmf-parser.dir/demo/GPMF_mp4reader.c.o -MF CMakeFiles/gpmf-parser.dir/demo/GPMF_mp4reader.c.o.d -o CMakeFiles/gpmf-parser.dir/demo/GPMF_mp4reader.c.o   -c ../demo/GPMF_mp4reader.c
../demo/GPMF_mp4reader.c: In function ‘OpenMP4Source’:
../demo/GPMF_mp4reader.c:124:16: error: storage size of ‘mp4stat’ isn’t known
  struct stat64 mp4stat;
                ^~~~~~~
../demo/GPMF_mp4reader.c:125:2: warning: implicit declaration of function ‘stat64’; did you mean ‘stat’? [-Wimplicit-function-declaration]
  stat64(filename, &mp4stat);
  ^~~~~~
  stat
[3/5] Building C object CMakeFiles/gpmf-parser.dir/demo/GPMF_print.c.o
../demo/GPMF_print.c: In function ‘printfData’:
../demo/GPMF_print.c:355:18: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 2 has type ‘int64_t {aka long int}’ [-Wformat=]
      DBG_MSG("%lld,", BYTESWAP64(*J));
               ~~~^
               %ld
../demo/GPMF_print.c:373:18: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘uint64_t {aka long unsigned int}’ [-Wformat=]
      DBG_MSG("%llu,", BYTESWAP64(*J));
               ~~~^
               %lu
[4/5] Building C object CMakeFiles/gpmf-parser.dir/GPMF_parser.c.o

@dnewman-gpsw
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed now.

Please sign in to comment.