SIGABRT in types.cpp Exiv2::DataBuf::alloc function #302
Milestone
Comments
|
The reproducer is not accessible, could you make it downloadable please? |
|
I think it's OK now! |
|
Reproducer SHA1: e43c1eb7134d2fc4c5548253477fd09d6b2fac79 Minimized sample (SHA1: 878078e9db9a8bdb875297d1d1131037dc996e8b) exiv2-issue-302.png.zip with afl-tmin: |
|
This issue was also solved by #316 |
|
@piponazo I am trying to backport the fix for CVE-2018-10958/CVE-2018-10999 to older versions of exiv2 in Debian (0.25, 0.24, and 0.23). I have prepared two patches based on commits 2fb00c8 and 3ad0050. In particular, I tried to avoid introducing the new enforce mechanism and instead tried to accomplish the same effect with if statements. I would appreciate it if you could review my work and provide any feedback on any adjustments that I might need to make. Patch 1: --- exiv2.git.orig/src/pngchunk.cpp
+++ exiv2.git/src/pngchunk.cpp
@@ -60,6 +60,7 @@
#include <iostream>
#include <cassert>
#include <cstdio>
+#include <algorithm>
/*
@@ -166,6 +167,9 @@
}
else if(type == iTXt_Chunk)
{
+ const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_-1], '\0');
+ if (nullSeparators < 2) throw Error(58);
+
// Extract a deflate compressed or uncompressed UTF-8 text chunk
// we get the compression flag after the key
--- exiv2.git.orig/src/error.cpp
+++ exiv2.git/src/error.cpp
@@ -105,7 +105,8 @@
{ 49, N_("TIFF directory %1 has too many entries") }, // %1=TIFF directory name
{ 50, N_("Multiple TIFF array element tags %1 in one directory") }, // %1=tag number
{ 51, N_("TIFF array element tag %1 has wrong type") }, // %1=tag number
- { 52, N_("%1 has invalid XMP value type `%2'") } // %1=key, %2=value type
+ { 52, N_("%1 has invalid XMP value type `%2'") }, // %1=key, %2=value type
+ { 58, N_("corrupted image metadata") }
};
}Patch 2: --- exiv2.git.orig/src/pngchunk.cpp
+++ exiv2.git/src/pngchunk.cpp
@@ -168,14 +168,24 @@
else if(type == iTXt_Chunk)
{
const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_-1], '\0');
- if (nullSeparators < 2) throw Error(58);
+ if (nullSeparators < 2) throw Error(58, "iTXt chunk: not enough null separators");
// Extract a deflate compressed or uncompressed UTF-8 text chunk
// we get the compression flag after the key
- const byte* compressionFlag = data.pData_ + keysize + 1;
+ const byte compressionFlag = data.pData_[keysize + 1];
// we get the compression method after the compression flag
- const byte* compressionMethod = data.pData_ + keysize + 2;
+ const byte compressionMethod = data.pData_[keysize + 2];
+
+ if (compressionFlag != 0x00 && compressionFlag != 0x01)
+ {
+ throw Error(58, "iTXt chunk: not valid value in compressionFlag");
+ }
+ if (compressionMethod != 0x00)
+ {
+ throw Error(58, "iTXt chunk: not valid value in compressionMethod");
+ }
+
// language description string after the compression technique spec
std::string languageText((const char*)(data.pData_ + keysize + 3));
unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
@@ -183,7 +193,7 @@
std::string translatedKeyText((const char*)(data.pData_ + keysize + 3 + languageTextSize +1));
unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());
- if ( compressionFlag[0] == 0x00 )
+ if ( compressionFlag == 0x00 )
{
// then it's an uncompressed iTXt chunk
#ifdef DEBUG
@@ -197,7 +207,7 @@
arr.alloc(textsize);
arr = DataBuf(text, textsize);
}
- else if ( compressionFlag[0] == 0x01 && compressionMethod[0] == 0x00 )
+ else if ( compressionFlag == 0x01 && compressionMethod == 0x00 )
{
// then it's a zlib compressed iTXt chunk
#ifdef DEBUG |
|
@rcsanchez97 Sorry for not getting back to you earlier, but your first patch has a small issue, instead of: it should be: (the The rest looks good to me. You can drop the strings from the throws in the second patch where you throw an error 58, as the string will be ignored in the exception message anyway. However, please test that your patches fix the issue by running exiv2 with asan. |
|
Hi @rcsanchez97 . I am also sorry for not replying before. I double checked the changes and the only issue I see is what @D4N pointed out already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I find this when I set ‘ulimit -v 1048576(1G)’.
The command is "exiv2 -et [poc]"
https://github.com/legend-issue/pocs/blob/master/exiv2/id:000004%2Csig:06%2Csrc:000036%2Cop:havoc%2Crep:128
The text was updated successfully, but these errors were encountered: