Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON parsing bug. #314

Closed
HCETeam opened this issue Oct 24, 2013 · 7 comments
Closed

JSON parsing bug. #314

HCETeam opened this issue Oct 24, 2013 · 7 comments
Assignees
Labels
Milestone

Comments

@HCETeam
Copy link

HCETeam commented Oct 24, 2013

Hello PocoTeam.

There is bug in last JSON implementation (poco-1.5.2-all).
Broken json structure lead to application crash with SIGSEGV signal, for example:
//------------------------------------ Simple source code example -------------------------
std::string jsonStr = "{"type":"10"}}";
Poco::JSON::Array::Ptr obj;
Poco::JSON::Parser parser;
Poco::Dynamic::Var result = parser.parse(jsonStr);
//-----------------------------------------------------------------------------------
Poco::JSON::Parser::parse() method call (in the 4th source string) causes SIGSEGV signal.
There don't throw any internal exceptions in parse() method call.
P.S We expect some kind of Poco::Exception throwing, for this bad JSON string - "{"type":"10"}}", but not SIGSEGV.
There is GDB stack (pocolib without debug info...):

Program received signal SIGSEGV, Segmentation fault.
0xb7be2567 in Poco::Dynamic::Var::~Var() () from /usr/local/lib/libPocoFoundation.so.22
Missing separate debuginfos, use: debuginfo-install glibc-2.17-18.fc19.i686 keyutils-libs-1.5.6-1.fc19.i686 krb5-libs-1.11.3-2.fc19.i686 libcom_err-1.42.7-2.fc19.i686 libevent-2.0.18-3.fc19.i686 libgcc-4.8.1-1.fc19.i686 libselinux-2.1.13-15.fc19.i686 libstdc++-4.8.1-1.fc19.i686 lzo-2.06-4.fc19.i686 mariadb-libs-5.5.32-8.fc19.i686 openssl-libs-1.0.1e-28.fc19.i686 pcre-8.32-7.fc19.i686 zlib-1.2.7-10.fc19.i686
(gdb) where
#0 0xb7be2567 in Poco::Dynamic::Var::~Var() () from /usr/local/lib/libPocoFoundation.so.22
#1 0xb7ab1f20 in Poco::JSON::ParseHandler::endObject() () from /usr/local/lib/libPocoJSON.so.22
#2 0xb7aaf16f in Poco::JSON::Parser::parse(std::string const&) () from /usr/local/lib/libPocoJSON.so.22
#3 0x08077781 in main (argc=1, argv=0xbfffefd4) at ./Source/main.cpp:836

@micheleselea
Copy link
Contributor

I tested the jsonStr you write on my Poco::JSON::Parser compiled for Win32 and it throw exception but does not crash

@HCETeam
Copy link
Author

HCETeam commented Nov 28, 2013

To micheleselea.

Please, can you tell me exception's type, that you fetch in Win32 environment. ?

@micheleselea
Copy link
Contributor

SyntaxException("JSON syntax error"); line 184 of Parser.cpp

@HCETeam
Copy link
Author

HCETeam commented Jan 14, 2014

It (SIGFAULT) looks like we call "_stack.pop()" for empty container in the ParseHandler.cpp:135 string.

@micheleselea
Copy link
Contributor

Correct, probably it's better to check for _stack.empty() before pop()
Anyway I think that in release mode I don't know if will generate a problem, because (speaking of win32 c++ version of stack) I think the empty() is checked internally before the operation and in debug cause a break of the software just to develop purpose

@HCETeam
Copy link
Author

HCETeam commented Jan 14, 2014

//STL docs.
(http://www.cplusplus.com/reference/stack/stack/pop/)
//This member function (std::stack::pop) effectively calls the member function pop_back of the underlying container object.
//->
(http://www.cplusplus.com/reference/deque/deque/pop_back/)
//Exception safety
//If the container is not empty, the function never throws exceptions (no-throw guarantee).
//Otherwise, the behavior is undefined.

We can't call std::stack::pop for empty containers, because - (...the behavior is undefined).
So, if Microsoft implementation has addition check inside std, that doesn't resolve/fix this bug.
For example compiled with gcc 4.7.2 falls with segfault under Linux OS.

@ghost ghost assigned aleks-f Jan 14, 2014
@micheleselea
Copy link
Contributor

You're right I agree with you, the check should be done before pop()

aleks-f added a commit that referenced this issue Apr 30, 2014
…tween value and newline

- fixed GH #314: JSON parsing bug
- added GH #313: MetaColumn additions for Data::ODBC and Data::SQLite
@aleks-f aleks-f closed this as completed Apr 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants