[asterisk-bugs] [JIRA] (ASTERISK-29614) app_agent_pool: XML Doc: unterminated entity reference

[Sean Bright (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-29614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=256127#comment-256127 ] 

Sean Bright edited comment on ASTERISK-29614 at 9/1/21 12:49 PM:
-----------------------------------------------------------------

I am probably not the right person to answer your questions. Everything I know about the ACO API I have learned while fixing this issue and trying to answer your questions. I do not have any extra insight into the reasons things were done the way they were.

There is no rule or policy about the name of the "general" section of a module's configuration. Different module authors chose different names.

bq. And, yes, in {{app_agent_pool}}, that section is called ‘generic’. However, the help works only for ‘global’ although I expect it to be ‘generic’.

Yes, I agree that it is confusing.

bq. And, yes, the help text was and is confusing: It mentions ‘global’ at the start and then shows that escaped style about ‘generic’ – was that Regular Expression before?

The code that prints the help text out assumes that the text from the {{category}} node is a regular expression - because everything was a regex before Corey's change - and prints it out {{/between slashes/}} to communicate that to the user. My patch turns {{ARRAY}} and {{EXACT}} matches back into regex-like syntax just for display in the {{config show help}} output.

bq. I have no idea what was and is intended (by the original code).

Neither do I.

bq. But I wonder, what exactly is expected to be shown to the user and what should be in the XML DOM.

All of this is done in {{xmldoc_update_config_type()}}. [It tries to find the {{configObject}} node associated with the type being registered|https://github.com/asterisk/asterisk/blob/6cc004dc5aa92bba2abd5bf3562c48340594ae6c/main/config_options.c#L1061]. If it can find it, [it creates a {{syntax}} node|https://github.com/asterisk/asterisk/blob/6cc004dc5aa92bba2abd5bf3562c48340594ae6c/main/config_options.c#L1071] underneath it and everything else is a child of that newly created {{syntax}} node. The generated DOM node ends up looking like (from {{app_confbridge}}):

{code:xml}
<syntax>
  <matchInfo>
    <category match="false">^general$</category>
    <field name="type">user</field>
  </matchInfo>
</syntax>
{code}

bq. Or asked differently: That variable {{category}} in {{ast_xml_set_text(.)}} should be just plain text. Nothing special, no further XPath pointer, no Regular Expression. Just text, to be shown to the user. Right?

-That is correct.- It is text that looks like a regular expression, which is why the help output uses {{=~}} and {{!\~}} which are typically used to indicate regular expression matching.

bq. Previously, it was a C array. Therefore, the confusion.

This is what this patch fixes. The code was trying to write a {{const char **}} as a string into the {{category}} node. This bug has existed since {{_ARRAY}} and {{_EXACT}} were added.

bq. Then it was no memory corruption but just a wrong type of data in that XML string.

Trying to write the {{const char **}} as a string caused the memory corruption. I don't know what the "wrong type of data in that XML string" means?

bq. And my initial analysis was incorrect. Was it?

Your initial analysis was that there was memory corruption. That is correct, it was memory corruption.


was (Author: seanbright):
I am probably not the right person to answer your questions. Everything I know about the ACO API I have learned while fixing this issue and trying to answer your questions. I do not have any extra insight into the reasons things were done they way they were.

There is no rule or policy about the name of the "general" section of a module's configuration. Different module authors chose different names.

bq. And, yes, in {{app_agent_pool}}, that section is called ‘generic’. However, the help works only for ‘global’ although I expect it to be ‘generic’.

Yes, I agree that it is confusing.

bq. And, yes, the help text was and is confusing: It mentions ‘global’ at the start and then shows that escaped style about ‘generic’ – was that Regular Expression before?

The code that prints the help text out assumes that the text from the {{category}} node is a regular expression - because everything was a regex before Corey's change - and prints it out {{/between slashes/}} to communicate that to the user. My patch turns {{ARRAY}} and {{EXACT}} matches back into regex-like syntax just for display in the {{config show help}} output.

bq. I have no idea what was and is intended (by the original code).

Neither do I.

bq. But I wonder, what exactly is expected to be shown to the user and what should be in the XML DOM.

All of this is done in {{xmldoc_update_config_type()}}. [It tries to find the {{configObject}} node associated with the type being registered|https://github.com/asterisk/asterisk/blob/6cc004dc5aa92bba2abd5bf3562c48340594ae6c/main/config_options.c#L1061]. If it can find it, [it creates a {{syntax}} node|https://github.com/asterisk/asterisk/blob/6cc004dc5aa92bba2abd5bf3562c48340594ae6c/main/config_options.c#L1071] underneath it and everything else is a child of that newly created {{syntax}} node. The generated DOM node ends up looking like (from {{app_confbridge}}):

{code:xml}
<syntax>
  <matchInfo>
    <category match="false">^general$</category>
    <field name="type">user</field>
  </matchInfo>
</syntax>
{code}

bq. Or asked differently: That variable {{category}} in {{ast_xml_set_text(.)}} should be just plain text. Nothing special, no further XPath pointer, no Regular Expression. Just text, to be shown to the user. Right?

-That is correct.- It is text that looks like a regular expression, which is why the help output uses {{=~}} and {{!\~}} which are typically used to indicate regular expression matching.

bq. Previously, it was a C array. Therefore, the confusion.

This is what this patch fixes. The code was trying to write a {{const char **}} as a string into the {{category}} node. This bug has existed since {{_ARRAY}} and {{_EXACT}} were added.

bq. Then it was no memory corruption but just a wrong type of data in that XML string.

Trying to write the {{const char **}} as a string caused the memory corruption. I don't know what the "wrong type of data in that XML string" means?

bq. And my initial analysis was incorrect. Was it?

Your initial analysis was that there was memory corruption. That is correct, it was memory corruption.

> app_agent_pool: XML Doc: unterminated entity reference
> ------------------------------------------------------
>
>                 Key: ASTERISK-29614
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-29614
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Applications/app_agent_pool, Applications/app_skel, Documentation
>    Affects Versions: 13.38.3, 16.20.0, 18.6.0, 19.0.0
>         Environment: Ubuntu 18.04 LTS
> ./configure --enable-xmldoc, which is the default
> make all or make full, does not matter
>            Reporter: Alexander Traud
>            Assignee: Sean Bright
>            Severity: Major
>
> This issue seems not to be of constant occurrence. Furthermore, I was not able to find any reports about this except several failed builds in Gerrit. Perhaps those are related but I did not investigated those further, yet. 
> Currently, I am able to replicate it quite constant and was able to investigate a bit with GDB. However, I am reporting early in my analysis, perhaps somebody sees the culprit faster.
> *My symptom*:
> {code} Loading app_agent_pool.so.
>   == Manager registered action Agents
>   == Manager registered action AgentLogoff
>   == Registered custom function 'AGENT'
>   == Registered application 'AgentLogin'
>   == Registered application 'AgentRequest'
> error : unterminated entity reference            ���
>   == app_agent_pool.so => (Call center agent pool applications){code}*My call stack*:
> apps/app_agent_pool.c
> ⤷ load_module
>    ⤷ load_config
>       ⤷ aco_info_init
>          ⤷ type = agent_type (the second type from {{app_agent_pool}}, the one after the {{general_type}})
>             ⤷ xmldoc_update_config_type in file main/xml.c
>                ⤷ ast_xml_set_text
>                   ⤷ xmlNodeSetContent
> That error is not printed, when I remove _all_ categories except the terminating NULL, in the file {{apps/app_agent_pool.c}}, in the string array {{agent_type_blacklist}}. That error is not printed, when I comment/disable at least four of the eight {{aco_option_register}} in {{load_config}}.
> This looks like a memory corruption, because even when that error is not printed, on the command-line interface (CLI), I am not able to issue {{xmldoc dump <file>}} when the module {{app_agent_pool}} is loaded. I get errors like {{output error : string is not in UTF-8}} or {{xmlEscapeEntities : char out of range}}.
> *Workarounds*:
> a) {{./configure --disable-xmldoc}} or
> b) disable the module {{app_agent_pool}} via {{make menuselect}} or
> c) noload the module {{app_agent_pool}} via the configuration file {{modules.conf}} or
> d) change the string array {{agent_type_blacklist}} to contain just the terminating {{NULL}} value



--
This message was sent by Atlassian JIRA
(v6.2#6252)