View Issue Details

IDProjectCategoryView StatusLast Update
0004174Talermerchant backendpublic2021-09-02 16:22
ReporterMarcello Stanisci Assigned ToFlorian Dold Resolution deadline
ImplementerReviewer 
Release signed byDeployed by 
VerifierCommunicator 
Data protection authority notification requiredNo  
CSIRT notification requiredNo  
Estimated effort (hours)Actual effort (hours)0 
Fast-trackNo
Work packagenone
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version0.0 
Target Version0.0Fixed in Version0.0 
Summary0004174: segfault if malformed JSON POSTed
DescriptionAlthough the backend returns a 'invalid json' message, it
segfaults anyways. No valgrind log attached since it just mentions
an 'Illegal instruction' error without telling where in the source code
...
Steps To ReproducePOST bogus JSON to any backend's API call accepting POSTed JSONs
TagsNo tags attached.

Required notifications

Address or linkRemarkAdded byNotifiedActions

Activities

Florian Dold

2016-02-17 00:42

manager   ~0010143

Does the merchant have a test harness right now so we can add some regression tests for this?

Also, might this be related to 0004109?

Florian Dold

2016-02-17 01:00

manager   ~0010144

Just FYI, here's the valgrind output:


Feb 17 01:59:18-376154 taler-merchant-httpd-30479 WARNING Failed to parse JSON request body
==30479== Jump to the invalid address stated on the next line
==30479== at 0xE132772: ???
==30479== by 0x566EF32: MHD_connection_handle_idle (connection.c:2750)
==30479== by 0x5673B49: MHD_run_from_select (daemon.c:2333)
==30479== by 0x5673EC2: MHD_select (daemon.c:2476)
==30479== by 0x567396A: MHD_run (daemon.c:3007)
==30479== by 0x402DD8: run_daemon (taler-merchant-httpd.c:311)
==30479== by 0x5AD7E15: run_ready (scheduler.c:587)
==30479== by 0x5AD7E15: GNUNET_SCHEDULER_run (scheduler.c:868)
==30479== by 0x5AD309D: GNUNET_PROGRAM_run2 (program.c:302)
==30479== by 0x5AD337E: GNUNET_PROGRAM_run (program.c:341)
==30479== by 0x403DF3: main (taler-merchant-httpd.c:593)
==30479== Address 0xe132772 is 30 bytes before a block of size 16 in arena "client"
==30479==
vex amd64->IR: unhandled instruction bytes: 0x2F 0x65 0x74 0x63 0x2F 0x6C 0x6F 0x63
vex amd64->IR: REX=0 REX.W=0 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR: VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR: PFX.66=0 PFX.F2=0 PFX.F3=0
==30479== valgrind: Unrecognised instruction at address 0xe132790.
==30479== at 0xE132790: ???
==30479== by 0x566EF32: MHD_connection_handle_idle (connection.c:2750)
==30479== by 0x5673B49: MHD_run_from_select (daemon.c:2333)
==30479== by 0x5673EC2: MHD_select (daemon.c:2476)
==30479== by 0x567396A: MHD_run (daemon.c:3007)
==30479== by 0x402DD8: run_daemon (taler-merchant-httpd.c:311)
==30479== by 0x5AD7E15: run_ready (scheduler.c:587)
==30479== by 0x5AD7E15: GNUNET_SCHEDULER_run (scheduler.c:868)
==30479== by 0x5AD309D: GNUNET_PROGRAM_run2 (program.c:302)
==30479== by 0x5AD337E: GNUNET_PROGRAM_run (program.c:341)
==30479== by 0x403DF3: main (taler-merchant-httpd.c:593)
==30479== Your program just tried to execute an instruction that Valgrind
==30479== did not recognise. There are two possible reasons for this.
==30479== 1. Your program has a bug and erroneously jumped to a non-code
==30479== location. If you are running Memcheck and you just saw a
==30479== warning about a bad jump, it's probably your program's fault.
==30479== 2. The instruction is legitimate but Valgrind doesn't handle it,
==30479== i.e. it's Valgrind's fault. If you think this is the case or
==30479== you are not sure, please let us know and we'll try to fix it.
==30479== Either way, Valgrind will now raise a SIGILL signal which will
==30479== probably kill your program.
==30479==
==30479== Process terminating with default action of signal 4 (SIGILL)
==30479== Illegal opcode at address 0xE132790
==30479== at 0xE132790: ???
==30479== by 0x566EF32: MHD_connection_handle_idle (connection.c:2750)
==30479== by 0x5673B49: MHD_run_from_select (daemon.c:2333)
==30479== by 0x5673EC2: MHD_select (daemon.c:2476)
==30479== by 0x567396A: MHD_run (daemon.c:3007)
==30479== by 0x402DD8: run_daemon (taler-merchant-httpd.c:311)
==30479== by 0x5AD7E15: run_ready (scheduler.c:587)
==30479== by 0x5AD7E15: GNUNET_SCHEDULER_run (scheduler.c:868)
==30479== by 0x5AD309D: GNUNET_PROGRAM_run2 (program.c:302)
==30479== by 0x5AD337E: GNUNET_PROGRAM_run (program.c:341)
==30479== by 0x403DF3: main (taler-merchant-httpd.c:593)
==30479==
==30479== HEAP SUMMARY:
==30479== in use at exit: 287,896 bytes in 3,517 blocks
==30479== total heap usage: 261,864 allocs, 258,347 frees, 8,270,260 bytes allocated
==30479==
==30479== LEAK SUMMARY:
==30479== definitely lost: 200 bytes in 1 blocks
==30479== indirectly lost: 0 bytes in 0 blocks
==30479== possibly lost: 1,086 bytes in 32 blocks
==30479== still reachable: 286,610 bytes in 3,484 blocks
==30479== suppressed: 0 bytes in 0 blocks
==30479== Rerun with --leak-check=full to see details of leaked memory
==30479==
==30479== For counts of detected and suppressed errors, rerun with: -v
==30479== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Illegal instruction

Florian Dold

2016-02-17 01:11

manager   ~0010145

It seems that the segfault only happens with /backend/hash-contract, posting invalid JSON to other endpoints works fine (even twice).

Florian Dold

2016-02-17 01:27

manager   ~0010146

Segfault also happens with /backend/contract ... (but not with /backend/pay)

Florian Dold

2016-02-17 02:06

manager   ~0010147

I've found the cause: The merchant backend was expecting the MHD connection_cls to have a certain format (a struct where the first field is a cleanup callback). The cleanup callback is executed for all requests.

This was violated by /backend/contact and /backend/hash-contract, and caused some unrelated pointer to be executed as cleanup callback.

This should be better documented in the code ...

Issue History

Date ModifiedUsernameFieldChange
2016-02-16 19:38Marcello StanisciNew Issue
2016-02-16 19:38Marcello StanisciStatusnew => assigned
2016-02-16 19:38Marcello StanisciAssigned To => Marcello Stanisci
2016-02-17 00:42Florian DoldNote Added: 0010143
2016-02-17 01:00Florian DoldNote Added: 0010144
2016-02-17 01:11Florian DoldNote Added: 0010145
2016-02-17 01:27Florian DoldNote Added: 0010146
2016-02-17 02:06Florian DoldNote Added: 0010147
2016-02-17 02:11Florian DoldStatusassigned => informing
2016-02-17 02:11Florian DoldResolutionopen => fixed
2016-02-17 02:11Florian DoldAssigned ToMarcello Stanisci => Florian Dold
2016-02-17 15:34Christian GrothoffProduct Version => 0.0
2016-02-17 15:34Christian GrothoffFixed in Version => 0.0
2016-02-17 15:34Christian GrothoffTarget Version => 0.0
2016-02-17 15:35Christian GrothoffStatusinforming => closed
2021-09-02 16:22Florian Dold
2025-03-22 13:09Christian GrothoffCategorymechant backend => merchant backend