Relationship Graph

Relationship Graph
related to related to child of child of duplicate of duplicate of

View Issue Details

This bug affects 1 person(s).
 6
IDProjectCategoryView StatusLast Update
16937Bug reportsSurvey takingpublic2021-07-20 13:48
Reportergabrieljenik Assigned Toollehar  
PrioritynormalSeverityminor 
Status ready for code reviewResolutionopen 
Product Version4.4.0-RC1 
Summary16937: Easy way to launch CDBException
Description

Really easy to launch a CDbException publicly

Steps To Reproduce

Import included survey
Copy the start link with newtest=Y
Add &Q00=ABCDEFGHIJ
Send the link

Additional Information

Seems prefilled value are not filtered with https://github.com/LimeSurvey/LimeSurvey/blob/bdd01aab12a6c9897cb777fbb8e3692a6f95858d/application/helpers/expressions/em_manager_helper.php#L10132

Here it's with mysql, but other have same issue (date for example)

The 2 htmlk are done with debug or without

Original solution for LS3 on 16481 needs to be ported or enhanced for LS4.

TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)4.4.0-RC1
I will donate to the project if issue is resolvedNo
Browsernot relevant
Database type & versionmariadb
Server OS (if known)fedora/linux
Webserver software & version (if known)nginx
PHP Versionphp7.3

Relationships

related to 16481 closedDenisChenu Easy way to launch CDBException 

Activities

gabrieljenik

gabrieljenik

2021-01-05 14:25

manager   ~61432

PR: https://github.com/LimeSurvey/LimeSurvey/pull/1708

gabrieljenik

gabrieljenik

2021-01-11 17:11

manager   ~61481

Last edited: 2021-01-13 16:48

@DenisChenu commented on the PR:

for master : we need a better solution : move after setMoveResult ?

We have moved it:

  • from StartSurvey(), which is run inside initFirstStep()
  • to after setMoveResult

This is the list of steps it does for starting up:

    /**
     * Init session/params values depending of user moves
     *
     * - It init the needed variables for navigation: initFirstStep, initTotalAndMaxSteps, setMoveResult
     * - Then perform all the needed checks before moving:
     *   + did the participant used browser navigation?
     *   + did he pressed clear cancel, is he a confirmed quota?
     *   + Is the previous step set?
     *   + Is the survey finished?
     *   + Are all the answer validated? (like: participant didn't answered to a mandatory question)
     */
    private function initMove()
    {
        $this->initFirstStep(); // If it's the first time user load this survey, will init session and LEM
        $this->initTotalAndMaxSteps();
        $this->checkIfUseBrowserNav(); // Check if user used browser navigation, or relaoded page
        if ($this->sMove != 'clearcancel' && $this->sMove != 'confirmquota') {
            $this->checkPrevStep(); // Check if prev step is set, else set it
            $this->setMoveResult();
            $this->checkClearCancel();
            $this->setPrevStep();
            $this->checkIfFinished();
            $this->setStep();
            $this->saveStartingValues();

Why do we need to move it after setMoveResult()?
Shouldn't it be done before so the EM data is available for setting the move?

DenisChenu

DenisChenu

2021-01-11 17:18

developer   ~61482

Last edited: 2021-01-13 16:48

To fix https://bugs.limesurvey.org/view.php?id=16482

prefilling value muts be done only when create survey by another way :)

DenisChenu

DenisChenu

2021-01-11 17:19

developer   ~61483

Last edited: 2021-01-13 16:48

PS :
Another solution : set it to defaultValue on EM ?

gabrieljenik

gabrieljenik

2021-01-12 21:55

manager   ~61507

Last edited: 2021-01-13 16:48

I understand now.
So now this ticket and 16482 (but for lsv4) are fixed.

This approach is OK for me!

DenisChenu

DenisChenu

2021-01-13 08:04

developer   ~61513

Last edited: 2021-01-13 16:48

Finally : i think you're right …

Use default value system are (maybe) not the best solution …

  • Prefill : value was set at start of survey even if variables are in last group
  • Default : value was set when step happen (for exemple : in last group : set just before submit).

I'm totally unsure on the best solution here … Using defaultvalue seems cool but unsure it the best solution.
https://github.com/LimeSurvey/LimeSurvey/blob/68ce18e22194171e1c56c27f36ad7ce5b34adc8a/application/helpers/expressions/em_manager_helper.php#L7126

gabrieljenik

gabrieljenik

2021-01-14 17:36

manager   ~61545

Ok, as to start fresh and avoid missunderstandings, I feel comfortable with the latest PR.
What do you think?

DenisChenu

DenisChenu

2021-01-14 17:42

developer   ~61546

if ($this->sSurveyMode != 'survey' && $_SESSION[$this->LEMsessid]['step'] == 0) return;

Yep : seems OK !

Maybe check $this->aSurveyInfo['showwelcome'] ? https://github.com/LimeSurvey/LimeSurvey/blob/79656ae39499fafd8a36c6ee12c5c9649d74df70/application/helpers/SurveyRuntimeHelper.php#L788

gabrieljenik

gabrieljenik

2021-01-24 22:36

manager   ~61657

New PR: https://github.com/LimeSurvey/LimeSurvey/pull/1726

ollehar

ollehar

2021-02-08 17:37

administrator   ~62034

Waiting for PR to master branch.

gabrieljenik

gabrieljenik

2021-02-08 20:48

manager   ~62051

New PR: https://github.com/LimeSurvey/LimeSurvey/pull/1754

ollehar

ollehar

2021-02-11 15:56

administrator   ~62173

About how to reproduce:

Import included survey
Copy the start link with newtest=Y
Add &Q00=ABCDEFGHIJ
Send the link

What happens after link is sent? What is the problem? I don't understand the issue title.

DenisChenu

DenisChenu

2021-02-11 17:15

developer   ~62180

500 error about DB

See original issue https://bugs.limesurvey.org/view.php?id=16481

ollehar

ollehar

2021-02-11 17:49

administrator   ~62182

Add &Q00=ABCDEFGHIJ

How this this an issue if the URL is manually hacked?

DenisChenu

DenisChenu

2021-02-11 17:55

developer   ~62183

Last edited: 2021-02-11 17:57

?

Admin user can update question type for example. Moving for a long text to a single choice

Can lauch a 500 erro, but i really think showing «CDbCommand failed to execute the SQL statement: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column '721221X2043X31723' at row 1» for public user is not the best way to do.

  1. Log it as error : Yes
  2. Throw an error or filter ? : Opinion, personally : i think it's better to filter silently. And if throw an error : send a 400 : bad params, not a 500 500 error can always be avoided.
ollehar

ollehar

2021-02-11 18:52

administrator   ~62186

Last edited: 2021-02-11 18:53

Admin user can update question type for example. Moving for a long text to a single choice

How exactly would this lead to a 500 exception? Genuine question.

DenisChenu

DenisChenu

2021-02-11 19:01

developer   ~62189

String data, right truncated …

There are a LSA in the other issue : https://bugs.limesurvey.org/view.php?id=16481#c61441

ollehar

ollehar

2021-02-11 19:03

administrator   ~62191

String data, right truncated …

WITHOUT hacking the URL. :D No sane user would hack the URL and expect the software to still work flawlessly.

DenisChenu

DenisChenu

2021-02-11 20:55

developer   ~62198

Admin user create some link with

ComeFrom=facebook
ComeFrom=twitter
ComeFrom=limesurvey

Send this link …

After : think : oh it's better if i have ComeFrom as single choice

Then :
ComeFrom=fb
ComeFrom=tw
ComeFrom=ls

No sane user would hack the URL and expect the software to still work flawlessly.

500 error still an issue …

Another situation : Email reader add a parameter automatically (or a proxy) etc …
A It's not a hacked string : it's just "more than 5 caracter" …

And since we know it's bad aram : wwe can lauch a 400, not a 500 …

No sane user would hack the URL and expect the software to still work flawlessly.

I don't care about the user entering url, i care about a clean error is show for the hoster.

gabrieljenik

gabrieljenik

2021-02-12 15:26

manager   ~62215

Calling checkValidityAnswer was missing on this version.
Will add it.

Then will try to reproduce the error again and see what kind of erro does it show.
We can pick it up later from there.

ollehar

ollehar

2021-02-23 11:23

administrator   ~62460

Considering the amount of blocking issues, maybe not spend too much time on this right now. ^^

Issue History

Date Modified Username Field Change
2020-12-31 15:24 gabrieljenik New Issue
2020-12-31 15:24 gabrieljenik Issue generated from: 16481
2020-12-31 15:25 gabrieljenik Additional Information Updated
2020-12-31 15:25 gabrieljenik Relationship added related to 16481
2021-01-05 14:25 gabrieljenik Note Added: 61432
2021-01-11 17:11 gabrieljenik Note Added: 61481
2021-01-11 17:18 DenisChenu Note Added: 61482
2021-01-11 17:19 DenisChenu Note Added: 61483
2021-01-12 21:55 gabrieljenik Note Added: 61507
2021-01-12 21:56 gabrieljenik Note Edited: 61507
2021-01-13 08:04 DenisChenu Note Added: 61513
2021-01-13 16:48 cdorin Sync to Zoho Project => |Yes|
2021-01-14 17:36 gabrieljenik Note Added: 61545
2021-01-14 17:42 DenisChenu Note Added: 61546
2021-01-24 22:36 gabrieljenik Note Added: 61657
2021-02-08 17:37 ollehar Assigned To => ollehar
2021-02-08 17:37 ollehar Status new => feedback
2021-02-08 17:37 ollehar Note Added: 62034
2021-02-08 20:48 gabrieljenik Note Added: 62051
2021-02-08 20:48 gabrieljenik Status feedback => assigned
2021-02-11 15:56 ollehar Note Added: 62173
2021-02-11 17:15 DenisChenu Note Added: 62180
2021-02-11 17:15 DenisChenu File Added: Capture d’écran du 2021-02-11 17-14-28.png
2021-02-11 17:49 ollehar Note Added: 62182
2021-02-11 17:55 DenisChenu Note Added: 62183
2021-02-11 17:57 DenisChenu Note Edited: 62183
2021-02-11 18:52 ollehar Note Added: 62186
2021-02-11 18:53 ollehar Note Edited: 62186
2021-02-11 19:01 DenisChenu Note Added: 62189
2021-02-11 19:03 ollehar Note Added: 62191
2021-02-11 20:55 DenisChenu Note Added: 62198
2021-02-12 15:26 gabrieljenik Note Added: 62215
2021-02-23 11:23 ollehar Note Added: 62460
2021-02-23 11:24 ollehar Priority none => normal
2021-02-23 11:24 ollehar Sync to Zoho Project Yes => |Yes|
2021-03-22 13:04 c_schmitz Severity crash => minor
2021-03-22 13:04 c_schmitz Sync to Zoho Project Yes => |Yes|
2021-07-20 13:48 gabrieljenik Status assigned => ready for code review