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

Invalid XAVP use causes a crash #511

Closed
que273 opened this issue Feb 16, 2016 · 2 comments
Closed

Invalid XAVP use causes a crash #511

que273 opened this issue Feb 16, 2016 · 2 comments

Comments

@que273
Copy link
Contributor

que273 commented Feb 16, 2016

The following code is accepted by the cfg parser and will run:

$xavp(test1=>stuff) = "america";   # Good
$xavp(test2->stuff) = "bahamas";  # Bad  - Hyphen instead of equals
$xavp(test3) = "cuba"; # Bad - No name given
xlog("L_WARN", "1: $xavp(test1)\n");
xlog("L_WARN", "2: $xavp(test2)\n");
xlog("L_WARN", "3: $xavp(test3)\n");
xlog("L_WARN", "4: $xavp(test1=>stuff)\n");
xlog("L_WARN", "5: $xavp(test2=>stuff)\n");
xlog("L_WARN", "6: $xavp(test3=>stuff)\n");

This prints:

1: <<xavp:0x7ff16f82c898>>
2: <null>
3: cuba
4: america
5: null
<null> or general protection fault/ core was generated......

The crash can be fixed by checking the child node type (incoming), but could there be more validation of the (x)avp variables.
I notice that $avp's can be treated as $xavps i.e. the following is accepted.
$avp(test4=>stuff) = "dominica";

Is the usage above intended to be acceptable?

que273 pushed a commit that referenced this issue Feb 16, 2016
- Fixes the crash reported in #511
(cherry picked from commit 19bb634ba043e74480c724fb7ed8a2dad43e8dda)
que273 pushed a commit that referenced this issue Feb 16, 2016
- Fixes the crash reported in #511
@miconda
Copy link
Member

miconda commented Feb 17, 2016

In the case of

$avp(test4=>stuff) = "dominica";

the name is just considered as a static string, not a structure field reference like for xavps.

It doesn't bother me that this is allowed, as all variables can have whatever they like as inner name and it is up to each to interpret it as they need.

Thanks for the fixes to the bugs you reported.

@miconda
Copy link
Member

miconda commented Mar 9, 2016

Related commits seems to be pushed now, if it is anything else to sort out, re-open.

@miconda miconda closed this as completed Mar 9, 2016
miconda pushed a commit that referenced this issue Jul 4, 2016
- Fixes the crash reported in #511
(cherry picked from commit 19bb634ba043e74480c724fb7ed8a2dad43e8dda)

(cherry picked from commit a84d800)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants